How do I DRY the following code?

R. Bernstein rocky at panix.com
Tue Dec 30 05:04:10 EST 2008


Steven D'Aprano <steve at REMOVE-THIS-cybersource.com.au> writes:

> On Mon, 29 Dec 2008 21:13:55 -0500, R. Bernstein wrote:
>
>> How do I DRY the following code? 
>>
>> class C():
> [snip code]
>
> Move the common stuff into methods (or possibly external functions). If 
> you really need to, make them private by appending an underscore to the 
> front of the name.
>
>
> class C():
>   def common1(self, *args):
>     return "common1"
>   def common2(self, *args):
>     return "common2"
>   def _more(self, *args):  # Private, don't touch!
>     return "more stuff"
>
>   def f1(self, arg1, arg2=None, globals=None, locals=None):
>       ... unique stuff #1 ...
>       self.common1()
>       ret = eval(args, globals, locals)
>       self._more()
>       return retval
>
>   def f2(self, arg1, arg2=None, *args, **kwds):
>       ... unique stuff #2 ...
>       self.common1()
>       ret = arg1(args, *args, **kwds)
>       self.common2
>       return retval
>
>   def f3(self, arg1, globals=None, locals=None):
>       ... unique stuff #3 ...
>       self.common1()
>       exec cmd in globals, locals
>       self.common2()
>       return None  # unneeded
>
>   def f4(self, arg1, globals=None, locals=None):
>       ... unique stuff #4 ...
>       self.common1()
>       execfile(args, globals, locals)
>       self._more()
>

I realize I didn't mention this, but common1 contains "try:" and more contains
"except ... finally". 

So for example there is 

self.start()
try: 
    res = func(*args, **kwds)   # this line is different
except 
   ...
finally:
   ...

Perhaps related is the fact that common1 and common2 are really
related and therefore breaking into the function into 3 parts sort of
breaks up the flow of reading one individually. 

I had also considered say passing an extra parameter and having a case
statement around the one line that changes, but that's ugly and
complicates things too.


>
> An explicit "return None" is probably not needed. Python functions and 
> methods automatically return None if they reach the end of the function.

"return None" is a perhaps a stylistic idiom. I like to put "returns"
at the end of a function because it helps (and Emacs) me keep
indentation straight. Generally, I'll put "return None" if the
function otherwise returns a value and just "return" if I don't think
there is  a useable return value.

I've also noticed that using pass at the end of blocks also helps me
and Emacs keep indntation straight. For example:

if foo():
   bar()
else
   baz()
   pass
while True:
      something
      pass

>
>
>
>
>
>> Above there are two kinds of duplication: that within class C and that
>> outside which creates an instance of the class C and calls the
>> corresponding method.
>
> Do you really need them? 

Perhaps, because they may be the more common idioms. And by having
that function there a temporary complex object can be garbage
collected and doesn't pollute the parent namespace. Is this a big
deal? I don't know but it doesn't hurt.

> If so, you're repeating yourself by definition. 
> That's not necessarily a problem, the stand-alone functions are just 
> wrappers of methods. You can decrease (but not eliminate) the amount of 
> repeated code with a factory function:
>
> def build_standalone(methodname, docstring=None):
>     def function(arg, arg2=None, globals=None, locals=None):
>         c = C()
>         return c.getattr(methodname)(arg, arg2, globals, locals)
>     function.__name__ = methodname
>     function.__doc__ = docstring
>     return function
>
> f1 = build_standalone('f1', "Docstring f1")
> f2 = build_standalone('f2', "Docstring f2")
> f3 = build_standalone('f3', "Docstring f3")

Yes, this is better than the lambda. Thanks!

>
> There's no way to avoid the repeated f1 etc.
>
> But honestly, with only three such functions, I'd consider that overkill.
>
Yeah, I think so too.
>
>> Lest the above be too abstract, those who want to look at the full
>> (and redundant) code:
>>
>>   http://code.google.com/p/pydbg/source/browse/trunk/api/pydbg/api/
> debugger.py
>
>
> You have parameters called Globals and Locals. It's the usual Python 
> convention that names starting with a leading uppercase letter is a 
> class. To avoid shadowing the built-ins, it would be more conventional to 
> write them as globals_ and locals_. You may or may not care about 
> following the convention :)

Ok. Yes, some earlier code used globals_ and locals_. Thanks. 
>
> I notice you have code like the following:
>
> if Globals is None:
>     import __main__
>     Globals = __main__.__dict__
>
>
> I would have written that like:
>
> if Globals is None:
>     Globals = globals()

Yes, that's better. Thanks.
>
> or even
>
> if Globals is None:
>     from __main__ import __dict__ as Globals
>
> You also have at least one unnecessary pass statement:
>
> if not isinstance(expr, types.CodeType):
>     expr = expr+'\n'
>     pass
>
> The pass isn't needed.
>
>
> In your runcall() method, you say:
>
> res = None
> self.start(start_opts)
> try:
>     res = func(*args, **kwds)
> except DebuggerQuit:
>     pass
> finally:
>     self.stop()
> return res
>
> This is probably better written as:
>
> self.start(start_opts)
> try:
>     return func(*args, **kwds)
> except DebuggerQuit:
>     return None
> finally:
>     self.stop()

See above for why I use pass. Thanks for the suggestions!
>
>
>
>
> -- 
> Steven



More information about the Python-list mailing list