[Tutor] please return flys in ointment

[Alan Gauld]

On 06/07/13 22:38, Jim Mooney wrote:

> naturally, lest I look dumb, I'd appreciate criticism,

OK, here are a few comments, take em or leave em...

> import sys
>
> # Data

<snip...>

> # Functions
> def make_triplets_from_input():

Why not pass in the prompt string as an argument?

>      '''Enter a positive integer. A list of triplets in the same order
> will be returned.
>      Raise ValueError to loop on non-integer input, or return 'zero'
> trigger-value of
>      'xxx' if all zeroes are entered. If input is okay, strip leading
> zeroes, then
>      zero-pad leftmost triplet to three characters, so all slices are
> triplets. Adjust
>      input for different Python versions.'''

Not strictly accurate - see comments below.

>      while True:
>          try:
>              if sys.version[0:3] == '2.7':
>                  numbers_str = original = raw_input('Enter a positive
> integer, space \
> separated if desired.')

I'd probably just use

input = raw_input

to keep the code simple.

>              elif sys.version[0:3] == '3.3':
>                  numbers_str = original = input('Enter a positive
> integer, space \
> separated if desired.')

Then you don't really need this bit unless you really want to
exclude other v3.X Python's which seems a tad harsh!

>              else:
>                  print('Python versions 2.7 and 3.3 only supported')
>                  sys.exit()

Probably better to take a chance and say Python versions above 2.7.
Backward compatibility is pretty good in Python land.


>              numbers_str = ''.join(numbers_str.split())
>              if numbers_str == '' or numbers_str == ' ': raise ValueError
>              numbers_int = int(numbers_str)

Why not just allow the built in Exception to raise itself?
 >>> int('')
Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: ''
 >>>

>              if numbers_int == 0:
>                  print('Zero')
>                  sys.exit()
>              numbers_str = str(numbers_int)

Not sure why this is needed, you already got the int from
the str so why not use the original string?

>              if len(numbers_str) > 36: raise ArithmeticError
>              break

Its not really Arithmetic? I'd have said a ValueError...

>          except KeyboardInterrupt:
>              print('Program cancelled by user')
>              sys.exit()

Again, what's wrong with the standard error message?

>          except ValueError as err:
>              print(original, "is not an integer.")
>              continue
>          except ArithmeticError as err:
>              print(original, "is too big.\n999...decillion is max:
> 10**37-1 or 36 chars \
> or 12 groups of 3 chars")

OK, Now i see why the ArithmeticError. But you cpuild just have printed 
the message before raising ValueError? Or even passed the message into 
ValueError?

 >>> raise ValueError('Value too big, try again')
Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
ValueError: Value too big, try again
 >>>

It feels like you are trying to do much of Python's job yourself.
Let the force be with you...

>      leftpad = len(numbers_str) % 3 # zero-pad left, so we get all
> 3-character triplets
>      if leftpad == 0: leftpad = ''
>      elif leftpad == 2: leftpad = '0'
>      else: leftpad = '00'

Feels like a job for a dictionary and get() - or even a
simple tuple index:

leftpad = ('','0','00')[leftpad]

Personally I prefer not to change types in a single
variable like that (probably my C/Pascal background
coming out!) so I'd probably do it like this:

leftpad = ('','0','00')[ len(numbers_str) % 3 ]

Or introduce an extra vareiable - padlen or somesuch...

>      numbers_str = leftpad + numbers_str
>      triplets = [numbers_str[x:x+3] for x in range(0,len(numbers_str),3)]
>      return (triplets, len(triplets))

Not sure I'd bother returning the len, the user can get it as needed 
very easily and your function name and docstring give no hint to expect 
a len result as well as the triplets.


No further comments - its getting late! :-)

HTH
-- 
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/