[Tutor] Key Error

[Alan Gauld]

"jim stockford" <jim at well.com> wrote

>> (The tests at the end are poorly written too. 
> 
> If you'd be willing to share your strong words, I'd
> be grateful to learn better alternatives.

OK, The strong words referred to the entire piece and 
the biggest error was the lack of try/except or get().
However the tests are badly done IMHO for several 
reasons, namely:

        if wssd<-989. or wspd<-989. or wmax<-989.: break
        if wspd==0.: break

1) Both sets of tests result in a break so the last test 
should be combined with the others in a single line.
(as a sub point I'd also prefer to see both tests on 
wspd placed together for ease of maintenance)

2) The values are floating point but the programmer has 
lazily omited the end zero which makes it very hard to 
spot the trailing decimal point.

3) The last equality test is inherently unreliable when 
using floating point values since if the value were derived 
by calciulation it might be 'nearly zero', the best way to 
check equality for floats is to test within a range. (If this 
case is guaranteed safe I would expect a comment to 
explain why, because it's such an unusual situation...)

4) The spacing between the < and minus sign is non 
existent which could lead to the combination being read 
as a <- arrow sign. Now inPyton that doesn't mean 
anything but in other languages does, so a reader 
not familiar with Python (like our OP) might be 
misdirected, whereas a simple space would remove 
any doubt.

Some might also object to the break being on the 
same line as the test but personally I don't object to 
that. But the points above all have real impact on legibility,
maintainability and, potentially, reliability.

We wouldn't normally expect to see such pickiness 
in this list because its aimed at beginners, but if this 
is production code, and from the look of it, in the 
telecomms market - which usually demands very high 
standards, then the quality should be much higher.

I'd prefer the tests to be written like this:

e = 0.0000000000001
if -e <= wspd <= e or 
   wspd  <  -989.0  or
   wssd  <  -989.0  or 
   wmax <  -989.0: 
      break

Its more typing and more room but less ambiguous 
and easier to read IMHO.


Alan G.