Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > comp.lang.python > #100104

Re: Brief Code Review Please - Learning Python

From Thomas 'PointedEars' Lahn <PointedEars@web.de>
Newsgroups comp.lang.python
Subject Re: Brief Code Review Please - Learning Python
Date 2015-12-07 21:21 +0100
Organization PointedEars Software (PES)
Message-ID <2734676.n8c9ur5snb@PointedEars.de> (permalink)
References <8fce9c71-a7bf-43fa-b8cc-f23c268e2b05@googlegroups.com> <mailman.2.1449423279.2247.python-list@python.org>

Show all headers | View raw


Joel Goldstick wrote:

> On Sun, Dec 6, 2015 at 12:21 PM, <qualityaddictorg@gmail.com> wrote:
>>     * 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.

How did you get that idea?

> Look up what is considered True in Python, and what is False.

ISTM that *you* should do that.

> I imagine you don't want quotes around True and False.

I imagine also that they do not want to write “== True” and “== False”.  
Because you can omit the former and should replace “x == False” with
“not x”.  This applies to many programming languages, even those that do 
type conversion (and have “!” instead of “not”).

> Any string will == True

No, Python does not do type conversion on comparison by default.

$ python -c 'print("" == False)'
False

$ python -c 'print("True" == True)'
False

$ python -c 'print(True == True)'
True

(You have to define the __eq__() method of an object for that, and then you 
should also define at least __ne__().)

>>  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

Watch for word wrap.  He sets the argument for the “multiline” parameter in 
the measure.get_font_metrics() method call from the “is_multi_lines” 
parameter of the measure_string() call.

Please trim your quotes to the relevant minimum.

OP: Please do not post code line numbers or any other "decoration" if you 
want a code review.  Especially with Python it is important that you post 
the code verbatim.

-- 
PointedEars

Twitter: @PointedEars2
Please do not cc me. / Bitte keine Kopien per E-Mail.$

Back to comp.lang.python | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Brief Code Review Please - Learning Python qualityaddictorg@gmail.com - 2015-12-06 09:21 -0800
  Re: Brief Code Review Please - Learning Python Joel Goldstick <joel.goldstick@gmail.com> - 2015-12-06 12:34 -0500
    Re: Brief Code Review Please - Learning Python Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2015-12-07 21:21 +0100
  Re: Brief Code Review Please - Learning Python Chris Angelico <rosuav@gmail.com> - 2015-12-07 04:49 +1100
  Re: Brief Code Review Please - Learning Python Peter Otten <__peter__@web.de> - 2015-12-06 19:14 +0100

csiph-web