suggestions for improving code fragment please

Steven D'Aprano steve+comp.lang.python at pearwood.info
Fri Mar 1 03:44:40 CET 2013


On Thu, 28 Feb 2013 19:47:12 +0000, The Night Tripper wrote:

> Hi there
>     I'm being very dumb ... how can I simplify this fragment?

I suggest that the best way to simplify that fragment is to change the 
design of your class so it isn't so horrible. As it stands now, your 
class defines an arbitrary number of params, which *may or may not exist*:

>         if arglist:
>             arglist.pop(0)
>             if arglist:
>                 self.myparm1 = arglist.pop(0)
>                 if arglist:
>                     self.myparm2 = arglist.pop(0)
>                     if arglist:
>                         self.myparm3 = arglist.pop(0)
>                         if arglist:
>                             self.parm4 = arglist.pop(0)
>         # ...

So using your class is a horrible experience:

if hasattr(instance, 'param1'):
    do_something_with(instance.param1)
else:
    fall_back_when_param1_doesnt_exist()
if hasattr(instance, 'param2'):
    do_something_with(instance.param2)
else:
    fall_back_when_param2_doesnt_exist()
if hasattr(instance, 'param3'):
    print "Do you hate this class yet?"



We have a perfectly good programming idiom to deal with a variable number 
of values: the list, or tuple if you prefer. So here's an alternative:

self.params = arglist[1:]

If order is not important:

self.params = set(arglist[1:])


If you have to map names to values:

self.params = dict(
    ('param%d' % i, value) for i, value in enumerate(arglist[1:])
    )


If you absolutely must have named attributes, I recommend that you choose 
a default value that indicates "missing". Conventionally, that is None, 
but you can always create your own sentinel value if needed:

SENTINEL = object()
arglist = arglist + [SENTINEL]*20
for i in range(1, 21):
    setattr(self, 'param%d' % i, arglist[i])



-- 
Steven



More information about the Python-list mailing list