Path: csiph.com!news.mixmin.net!newsreader4.netcologne.de!news.netcologne.de!newsfeed0.kamp.net!newsfeed.kamp.net!fu-berlin.de!uni-berlin.de!not-for-mail
From: Joel Goldstick <joel.goldstick@gmail.com>
Newsgroups: comp.lang.python
Subject: Re: Brief Code Review Please - Learning Python
Date: Sun, 6 Dec 2015 12:34:31 -0500
Lines: 95
Message-ID: <mailman.2.1449423279.2247.python-list@python.org>
References: <8fce9c71-a7bf-43fa-b8cc-f23c268e2b05@googlegroups.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
In-Reply-To: <8fce9c71-a7bf-43fa-b8cc-f23c268e2b05@googlegroups.com>
Precedence: list
Xref: csiph.com comp.lang.python:100058

On Sun, Dec 6, 2015 at 12:21 PM, <qualityaddictorg@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