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