How do I DRY the following code?

[R. Bernstein]

How do I DRY the following code?

class C():

def f1(self, arg1, arg2=None, globals=None, locals=None):
... unique stuff #1 ...
... some common stuff #1 ...
ret = eval(args, globals, locals)
... more stuff #2 ...
return retval

def f2(self, arg1, arg2=None, *args, **kwds):
... unique stuff #2 ...
... some common stuff #1 ...
ret = arg1(args, *args, **kwds)
... more common stuff #2 ...
return retval

def f3(self, arg1, globals=None, locals=None):
... unique stuff #3 ...
... some common stuff #1 ...
exec cmd in globals, locals
... more common stuff #2 ...
return None

def f4(self, arg1, globals=None, locals=None):
... unique stuff #4 ...
... some common stuff #1 ...
execfile(args, globals, locals)
... more stuff #2 ...
return None



f1(...):
"Docstring f1"
c = C()
return c.f1(...)

f2(...):
"Docstring f2"
c = C()
return c.f2(...)

f3(...):
"Docstring f3"
c = C()
return c.f3(...)


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.

For the outside duplication, I considered trying:

_call_internal = lambda name, *args, **kwds \
c = C() \
fn = getattr(c, name) \
return fn(*args, **kwds)

f1 = lambda arg, arg2=None, globals=None, locals=None: _call_internal('f1', ...)
f1.__doc__ = """ Docstring f1 """

f2= lambda arg, arg1=None, arg2, *args, **kwds: _call_internal('f2', ...)
f2.__doc__ = """ Docstring f2 """

However this strikes me as a little bit cumbersome, and harder
understand and maintain than the straightforward duplicated code.

Thoughts?

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

 

[Patrick Mullen]

How do I DRY the following code?

class C():

def f1(self, arg1, arg2=None, globals=None, locals=None):
... unique stuff #1 ...
... some common stuff #1 ...
ret = eval(args, globals, locals)
... more stuff #2 ...
return retval

Possibly make a separate method with the common stuff. Make each
variant function call various other methods with the "different stuff"
being different arguments to shared functions, or calling slightly
different methods. Take the code one issue at a time, this seems
complex enough that there is no easy, quick refactoring that can be
done.

Something like this is what I see, but it's hazy:

def f1(self,args):
... unique stuff #1
self.commonstuff1()
ret = self.myeval(args)
self.commonstuff2()
return retval

Looking at the actual code, my thoughts are that you pick the most
common base "run" type and stick with that, making other run types
modify that. For instance, you could pass an evaluator function in to
the base run() method, and call that, instead of having a separate
case for exec or eval. runfunc would pass in the function itself as
the evaluator. So it looks more like this:

def run(blah, blah, BLAH, BLAAAAAH, evaluator, locals, globals):
do much stuff
evaluator(code,locals,globals)
do much other stuff
def runfunc(blah, func):
run(blah, value_defined_for_runfunc, func, locals, globals)
def runeval(blah):
run(blah, value_defined_for_runfunc, eval, locals globals)

The thing I don't like about this is it could very well make the main
run function even hairier than it already is. But I don't think it
has to be that way.

This may not be the ideal solution either, if you can get a nice set
of lower-level methods which each run method compose in different ways
to produce the final result.

f1(...):
"Docstring f1"
c = C()
return c.f1(...)

f2(...):
"Docstring f2"
c = C()
return c.f2(...)

Why not just do either this:
C().f2(..) where you need f2

Or make the function an argument:
f(name):
c = C()
return getattr(c,name)()

I'm not sure why you need the top-level functions though. If you
really need them, and don't want to add an extra argument, I don't
think there is a simpler way to do than you have.

 

[Steven D'Aprano]

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()


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

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? 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")

There's no way to avoid the repeated f1 etc.

But honestly, with only three such functions, I'd consider that overkill.

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

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()

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()

 

[R. Bernstein]

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

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.

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!

 

[R. Bernstein]

Why not just do either this:
C().f2(..) where you need f2

Yes, this is a little better. Thanks!