critique my code, please
Scott David Daniels
scott.daniels at acm.org
Mon Feb 6 13:48:06 EST 2006
Brian Blais asked for suggestions and critiques of his code.
For style, look at:
http://www.python.org/peps/pep-0008.html
This is how I'd rewrite the first class:
YESNO = ('No', 'Yes')
class SimulationParams(object):
'''Simulation Parameters setting up of a simulation'''
def __init__(self):
'''Build a new simulation control object'''
self.epoch_number = 500
self.iter_per_epoch = 500
self.epoch_per_display = 1
self.random_seed = 'clock'
self.keep_every_epoch = 0
self.save_input_vectors = 0
def __repr__(self):
return '''Epoch Number: %s
Iter Per Epoch: %s
Epoch Per Display: %s
Random Seed: %s
Keep Every Epoch: %s
Save Input Vectors: %s
''' % (self.epoch_number, self.iter_per_epoch,
self.epoch_per_display, self.random_seed,
YESNO[self.keep_every_epoch],
YESNO[self.save_input_vectors])
Notes:
The docstrings I am using are too generic. You know your app, so
do something more app-appropriate.
True is a version of 1, and False is a version of 0, so a dictionary
like {0:"No",1:"Yes",True:"Yes",False:"No"} is actually length 2.
I'd just use a constant mapping available anywhere.
'a %s b' % 13 is 'a 13 b', so no need for your seed type test. If you
do want to make the type visually obvious, use %r rather than %s for
the seed.
Personally I'd choose shorter names, but this is more taste:
class SimulationParams(object):
'''Simulation Parameters setting up of a simulation'''
def __init__(self):
'''Build a new simulation control object'''
self.epoch = 500
self.iter_per_epoch = 500
self.epoch_per_display = 1
self.seed = 'clock'
self.keep_epochs = False # Since these seem to be booleans
self.save_input = False # Since these seem to be booleans
...
You also might consider making the __init__ provide defaulted args so
getting a different initial setup only requires your changing the setup:
...
def __init__(self, epoch=500, iter_per_epoch=500,
epoch_per_display=1, seed='clock',
keep_epochs=False, save_input=False):
'''Build a new simulation control object'''
self.epoch = epoch
self.iter_per_epoch = iter_per_epoch
self.epoch_per_display = epoch_per_display
self.seed = seed
self.keep_epochs = keep_epochs
self.save_input = save_input
...
More some other time.
--Scott David Daniels
scott.daniels at acm.org
More information about the Python-list
mailing list