Brief Code Review Please - Learning Python
Joel Goldstick
joel.goldstick at gmail.com
Sun Dec 6 12:34:31 EST 2015
On Sun, Dec 6, 2015 at 12:21 PM, <qualityaddictorg at gmail.com> wrote:
> [Note: Tried sending this twice to the Python-List mail list but I never
> saw it come through to the list.]
>
> Hello everyone,
>
> I am beginning to learn Python, and I've adapted some code from Google
> into the function below. I'm also looking at the PEP 8 style guide. I'd
> appreciate a brief code review so I can start this journey off on a solid
> foundation. :)
>
> * I've already changed my editor from tabs to 4 spaces.
> * I've also set my editor to alert me at 72 characters and don't
> exceed 79 characters.
> * I've named the function and variables with lower case, underscore
> separated, and meaningful words.
> * I've surrounded this function with two blank lines before and after.
> * I just recognized I need to pick a quote style and stick to it so
> I'll fix that (" " or ' ').
> * I'm not sure about the WidthAndHeight on lines 16 and 17 regarding
> capitalization.
>
You don't need the parens. If you feel it is easier to understand with
them, they won't hurt
> If I understand correctly a namedtuple is a class so the CapWords
> convention is correct.
> * Is how I've aligned the function arguments on lines 1-4 and 22-25
> good style or \
> is spreading these out onto fewer lines preferred?
>
If the parameters fit on one line, I think that is more common
> * Same question as right above but with the if tests on lines 5-8.
> * I've also used ( ) around the if tests, but I'm not sure if this is
> good Python style or not.
>
> 1 def measure_string(desired_text,
> 2 desired_font_family,
> 3 desired_font_size,
> 4 is_multi_lines):
> 5 if (desired_text != '') and \
> 6 (desired_font_family != '') and \
> 7 (desired_font_size != '') and \
> 8 ((is_multi_lines == "True") or (is_multi_lines == "False")):
>
The above test will always be True. Look up what is considered True in
Python, and what is False. I imagine you don't want quotes around True and
False. Any string will == True
> 9 with Image(filename='wizard:') as temp_image:
> 10 with Drawing() as measure:
> 11 measure.font_family = desired_font_family
> 12 measure.font_size = desired_font_size
> 13 measures = measure.get_font_metrics(temp_image,
> 14 desired_text,
> 15
> multiline=is_multi_lines)
>
No need to set multiline = ... when is_multiline is already defined
> 16 WidthAndHeight = namedtuple("WidthAndHeight", "Width Height")
> 17 width_and_height = WidthAndHeight(measures.text_width, \
> 18 measures.text_height)
> 19 return width_and_height
> 20
> 21
> 22 print(measure_string("some text\na new line",
> 23 "Segoe UI",
> 24 40,
> 25 "True"))
>
> Any and all feedback is much appreciated. As I said, I'm just beginning
> to learn Python and want to start off with a solid foundation. Thank you
> to everyone in advance for your time and thoughts.
>
> Jason
> --
> https://mail.python.org/mailman/listinfo/python-list
>
--
Joel Goldstick
http://joelgoldstick.com/stats/birthdays
More information about the Python-list
mailing list