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/
_______________________________________________
Tutor maillist - Tutor@python.org
To unsubscribe or change subscription options:
http://mail.python.org/mailman/listinfo/tutor
