Approach for refactoring a function that has lots of special braching

Question

I am writing a function that operates slightly different depending on the data passed in. The common and unique code is mingled. I don't like what I have and am looking for a better way. Hopefully, this case demonstrates the situation. Consider that each print line below is just a proxy for one or more lines of more complex code.

def myFunc(a, b):
    if a=='mode1':
        print "special mode stuff"
    print "do for any mode"
    if a=='mode1':
        print "special mode stuff"
    print "do some more stuff for any mode"
    if a=='mode1':
        print "special mode stuff"

Can you suggest a strategy to handle this better? I'll update my example if its insufficient to explain what I'm trying to accomplish.

Answer 1

In reality, the function may be a hundred lines with these 'ifs' sprinkled through the code.

Delegation is often used to handle this kind of situation, where you've got a set of operations that need to happen but you also want to provide opportunities to customize the behavior at various points along the way. Define a delegate interface with a method for each one of those mode1 special case blocks, and replace each of the blocks with calls to the delegate:

def myFunc(a, b):
    if delegate != nil:
        delegate.doSpecialThing()
    print "do for any mode"
    if delegate != nil:
        delegate.doAnotherSpecialThing()

Then create a class that implements all those delegate methods (doSpecialThing(), doAnotherSpecialThing(), etc.) as appropriate for the mode1 case. I'm guessing that you want a single object to be able to dynamically change its behavior depending on what "mode" it's in, and delegation makes that possible. If you have other modes like mode2 and mode3, you can change the object's behavior by just giving it a different delegate, or removing its delegate altogether to get back to just the standard behavior.

You can find plenty of concrete examples in the Apple's Cocoa and Cocoa Touch frameworks, which use delegation all over the place to customize the behavior of certain objects. It's not so common in those frameworks to swap in different delegates, but there's nothing that prevents it either.

Answer 2

The Strategy Pattern is used for these kinds of problems. In python, I would probably start out with simply using function pointers. You have a lot of options of how attack this in Python but here's one possible answer:

def myFunc(a, b):
  try:
    modeFunc = locals()[a]
  except:
    modeFunc = standard

  return modeFunc(b)

and then you'd have something like this:

def standard(b)
  print "do for any mode"
  print "do some more stuff for any mode"

def mode1(b)
  print "special mode stuff"
  print "do for any mode"
  print "special mode stuff"
  print "do some more stuff for any mode"

The obvious issue here is that you have common code interspersed throughout. If this is really common and has to happen across the board you might want to go with the Template Pattern as pointed out by gnat. It's basically the Strategy pattern but perhaps with more structure.

It's really hard to say whether to stick to the simple approach (and accept some redundancy) or go with something more structured. Personally, I'd choose the one that required less work which tends to be the simple one and then refactor if you end up with a maintenance.

Another option is to do both. That is, use function pointers and sometimes they might point to a template pattern object or other times they just point to a simple method. This is a good option if you have common logic with exceptions. That is, if you have conditions like this:

if a != "mode6":
  print "do some more stuff for any mode except mode 6"

Answer 3

Risk of down votes not withstanding, I would approach this by copy and paste, not by patterns and abstraction.

def myFunc (a,b):
    if a == 'mode1':
        # copy of entire original function body
    else:
        # another identical copy

There's now lots of dead branches from the nested conditionals. So delete the dead code below the top level if - programmatically if you can, manually if necessary.

You now have exactly one instance of this conditional remaining and can more easily see common code on the branches. Cut new abstractions at this point, e.g. define local functions.

This is a standard code transform - move the if up the call stack, deleting dead branches as the occur. It looks a bit strange to introduce duplication prior to removing duplication, but lots of the same conditional branch suggest it's the way to go here.

Answer 4

Solving this through inheritance:

I would refactor every if (someCondidiont) {doSomething()} into one method in a base class and let inheritance replace the branching:

baseclass has

def myFunc(a, b):
    doTask1(c,d)
    doTask2(c,d)

Mode1, Mode2, ... become derived classes that implement the doTaksX in different or doNothing ways.