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

Groups > comp.lang.python > #100104

Re: Brief Code Review Please - Learning Python

Show key headers only | 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 | Next — Previous 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