Steven D'Aprano <st...@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 -- http://mail.python.org/mailman/listinfo/python-list