qualityaddict...@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.
> 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?
> * 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")):
> 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)
> 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
Use short variable names - the "desired_" prefix does not carry much
information.
Avoid unrelated but similar names: measure or measures -- what's what?
Use consistent names. This is a bit tricky, but I would prefer multiline
over is_multi_lines because the underlying library uses the former.
Use the right datatype: multiline should be boolean, i. e. True or False,
not "True" or "False".
Provide defaults if possible: if the multiline argument is missing you can
scan the text for "\n" and set the flag accordingly
Prefer parens for line continuations
(a and
b)
over backslashes
a and \
b
Move code that only should be executed once from the function to the global
namespace: the namedtuple definition should be on the module level.
Use qualified names if you are referring library code: wand.drawing.Drawing
instead of just Drawing.
Avoid spurious checks, but if you do argument validation fail noisyly, i. e.
"wrong":
def f(fontname):
if is_valid(fontname):
return meaningful_result()
# implicitly return None
"correct":
class InvalidFontname(Exception):
pass
def f(fontname):
if not is_valid(fontname):
raise InvalidFontname(fontname)
return meaningful_result()
With changes along these lines your code might become
from collections import namedtuple
import wand.drawing
import wand.image
Extent = namedtuple("Extent", "width height")
def get_text_extent(
text, font_family, font_size, multiline=None):
"""Determine width and heights for `text`.
"""
if multiline is None:
multiline = "\n" in text
with wand.image.Image(filename='wizard:') as image:
with wand.drawing.Drawing() as measure:
measure.font_family = font_family
measure.font_size = font_size
metrics = measure.get_font_metrics(
image, text, multiline=multiline)
return Extent(
width=metrics.text_width,
height=metrics.text_height)
if __name__ == "__main__":
print(
get_text_extent(
"some text\na new line",
"Segoe UI",
40))
--
https://mail.python.org/mailman/listinfo/python-list
