[Tutor] please return flys in ointment

[Dave Angel]

On 07/06/2013 06:34 PM, Dave Angel wrote:
> On 07/06/2013 05:38 PM, Jim Mooney wrote:
>> I reworked my numbers_to_name program so I can actually follow the logic.
>> Since I think the best way to learn is to follow something through to
>> completion, I'll put it online so I learn how to do that. But naturally,
>> lest I look dumb, I'd appreciate criticism, improvements of bad form,
>> or if
>> anyone can break it, let me know ;') . Seems to work with a bit of random
>> testing, but ya never know. If you did, Msoft wouldn't be putting out
>> those
>> updates all the time. Works with Py 27 or 33, but that brings up a
>> question: I think most of my programs work with both if I don't do
>> anything
>> exotic, except for input(), which is the fly in the ointment. Instead of
>> using the routine below, could I redefine input() to use the routine,
>> or it
>> that a bad idea?
>>
>> I have a feeling the leftpad routine could be simplified, but it isn't
>> coming to me.
>>
>
> General comments:
>
> 1) several lines wrapped in the newreader, so I had to paste things
> together.  Presumably this won't happen whereever you put it online, but
> you should make sure and check after uploading it to make sure no such
> problems were introduced.
>
> Also, instead of using the backslash line-continuation character, I
> generally try to use the syntax of the language instead.  So if you have
> a quote which won't fit on one line, but you don't want embedded
> newlines in it either, you can use the implicit concatenation of
> adjacent literals.  Frequently you have to put parens around the whole
> thing to make sure the compiler sees it as one line.  But I think it's
> usually more readable that way.
>
> First example:  the prompt in the input/raw_input lines.
>
>
>> # Using C:\Python33\python.exe on Win 7 in c:\python33\jimprogs - not
>> Windows-specific
>> # Also works with python 2.7
>>
>
> You always take the data in by input/raw_input.  Why not permit a value
> on the command line?  And even more important, you're mixing
> input/output with algorithm here.
>
>> import sys
>>
>> # Data
>> ones = {'1': 'one', '2': 'two', '3': 'three', '4': 'four', '5': 'five',
>> '6': 'six',
>> '7': 'seven', '8': 'eight', '9': 'nine'}
>>
>> tens = {'2': 'twenty', '3': 'thirty', '4': 'forty', '5': 'fifty', '6':
>> 'sixty',
>> '7': 'seventy', '8': 'eighty', '9': 'ninety'}
>>
>> doubles = {'0': 'ten', '1': 'eleven', '2': 'twelve', '3': 'thirteen',
>> '4':
>> 'fourteen',
>> '5': 'fifteen', '6': 'sixteen', '7': 'seventeen', '8': 'eighteen', '9':
>> 'nineteen'}
>>
>> powers_of_1000 = (' thousand', ' million', ' billion', ' trillion',
>> ' quadrillion', ' quintillion', ' sextillion', ' septillion', '
>> octillion',
>> ' nonillion', ' decillion')
>>
>> '''add these later, and also option for dollars-and-cents ending.
>> 'vigintillion', 'novemdecillion', 'octodecillion', 'septendecillion',
>> 'sexdecillion', 'quindecillion', 'quattuordecillion', 'tredecillion',
>> 'duodecillion', 'undecillion',
>> 'decillion', 'nonillion'
>
> Those last two you've already implemented
>
>> '''
>>
>> # Functions
>> def make_triplets_from_input():
>>      '''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.'''
>>      while True:
>>          try:
>>              if sys.version[0:3] == '2.7':
>
> I'd do the version checking in initialization code, not here.  And I'd
> do it by redefining input to do what it does in 3.3
>
>>                  numbers_str = original = raw_input('Enter a positive
>> integer, space \
>> separated if desired.')
>>              elif sys.version[0:3] == '3.3':
>>                  numbers_str = original = input('Enter a positive
>> integer,
>> space \
>> separated if desired.')
>
> Note:  you explicitly support zero, so this should say  non-negative,
> rather than positive.
>
> I'm not sure why you support spaces between the digits, and not commas,
> but no biggie.  BTW, you also support other white space, like tabs.
>
>>              else:
>>                  print('Python versions 2.7 and 3.3 only supported')
>>                  sys.exit()
>>
>>              numbers_str = ''.join(numbers_str.split())
>>              if numbers_str == '' or numbers_str == ' ': raise ValueError
>>              numbers_int = int(numbers_str)
>>              if numbers_int == 0:
>>                  print('Zero')
>>                  sys.exit()
>
> This is just weird.  You're in the middle of an input routine, and you
> just print something and exit the code??
>
>
>>              numbers_str = str(numbers_int)
>>              if len(numbers_str) > 36: raise ArithmeticError
>>              break
>>          except KeyboardInterrupt:
>>              print('Program cancelled by user')
>>              sys.exit()
>>          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")
>>
>>      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'
>
> Instead of these four lines, how about:
>         leftpad = "0" * (-i)%3
>
>>      numbers_str = leftpad + numbers_str
>>      triplets = [numbers_str[x:x+3] for x in range(0,len(numbers_str),3)]
>>      return (triplets, len(triplets))
>
> Seems a little silly to make this a tuple, when the second value is
> trivially figured from the first.  You could just have the second
> function take a single list argument and take its len() for itself.
>
> And even then, I'd recommend using a slice when calling it instead of
> passing it a number.  Just one more bug lurking in the wings.
>>
>> def numbers_to_name(triplets, triplen):
>>      '''Create a name from each triplet and append the power of ten'''
>>      triplen -= 2
>>      number_name = ''
>>
>>      for triplet in triplets:
>>          triplet_name = ''
>>          first, second, third, last_two = (triplet[0], triplet[1],
>> triplet[2], triplet[1:])
>>          if triplet == '000':
>>              triplen -= 1
>>              continue
>>          if first > '0':
>>              if last_two == '00': # special case - snip extra space
>> separator
>>                  triplet_name += ones.get(first) + ' hundred'
>>              else:
>>                  triplet_name += ones.get(first) + ' hundred '
>>          if second == '0':
>>              if third > '0':
>>                  triplet_name += ones.get(third)
>>          elif second == '1':
>>              triplet_name += doubles.get(third)
>>          elif second > '1':
>>              triplet_name += tens.get(second)
>>              if third > '0':
>>                  triplet_name += '-' + ones.get(third)
>>          number_name += triplet_name
>>          if triplen > -1:
>>              number_name += powers_of_1000[triplen] + ', '
>>          triplen -= 1
>>      if number_name[-2] == ',':
>>          number_name = number_name[0:-2] # special case - snip extraneous
>> ending comma
>>      return number_name
>>
>> # Main Program - turn numeric input into number-names
>> triplets, triplen = make_triplets_from_input()
>> print(numbers_to_name(triplets, triplen))
>>
>
> If you included the usual if __name__ ==  test around those two lines,
> you'd be able to use this same file as an importable module.
>
> You should have four functions, not two:
>
> 1) parse argv
> 2) get input from user
> 3) turn string into list  ( make_triplets_from_input)
> 4) turn list into string, ready to print  (numbers_to_name)
>
> and then the top-level code does something like:
>
> if __name__ == "__main__":
>      if len(argv) == 2:
>          raw = parseargv()
>      elif len(argv) == 1:
>          raw = getinputfromuser()
>      else:
>             someerrormessage
>             exit
>      triplets =  make_triplets_from_input(raw)
>      print(numbers_to_name(triplets))
>
>

I see that I forgot to include the input/raw_input logic.  I'd put in 
your source, right after imports:


#No need to check version, just try to use raw_input.  If it's not defined,
#  then input should already work
try:
     input = raw_input
except NameError as e:
     pass      #we must be on version 3.3

Once those lines are executed, just pretend you're on 3.3, and use input.

In numbers_to_name(), you can start with:

     if triplets == ["000"]:
         return "zero"

which will take care of the zero case where it belongs, not in the input 
function.  Incidentally, I changed it to lower case to match the rest of 
the values.

BTW, you use the word billion for 10**9.  That's good in the US.  But in 
England at least, a billion is 10**12.  So you should qualify that your 
program is intended for US word-numbers.

With the refactoring I suggested, I now can test the code with a simple 
loop:


for i in range(100000):
     triplets = make_triplets_from_input(str(i))
     print(i, numbers_to_name(triplets))

I pipe the output into 'less' and I can examine all those values 
quickly.  I could also write a much more exhaustive test, testing 
various assertions about the results.  And of course I can trivially 
write/generate a test suite with a list of maybe 10,000 values to check, 
and use that test after every change to check for regressions.

More later.

-- 
DaveA