How to avoid code duplication when else and except are the same?

Question

A simplified version of my code looks like this:

def process( object ):
 try:
 if suitedForA( object ):
 try:
 methodA( object )
 except:
 methodB( object )
 else:
 # we would get a bad result from methodA()
 methodB( object )
 except:
 methodC( object )

Edit: Removed error in pseudo code pointed out by @gnasher729.

I would like to avoid the repetition of methodB(). It may look not as bad in above pseudo-code, but in actuality, these are library calls with a bit of post-processing.

Edit: methodA/B/C() may fail without me being able to tell when. This is independent of suitedForA().

The following code avoids the repetition, but I am not sure if it is a good idea to throw an exception for this case:

def process( object ):
 try:
 if not suitedForA( object ):
 raise NotSuitedException()
 methodA( object )
 except:
 try:
 methodB( object )
 except:
 methodC( object )

Is there a simpler approach to achieve the same without the repetition and without actively throwing an exception?

• Ewan: dont use exceptions for flow control
• Kevin: Can methodB(object) throw an exception? If it can, you may end up calling it twice in your first function, but only once in your second function. Maybe your try/except should be inside the if suitedForA(object):?
• Greg Burghardt: Can you edit your question to include more details? These generic names make it difficult to answer your question. @Kevin brings up a good point about possibly calling methodB twice. Depending on what these functions do, this might be OK, but it also might be a bug.
• JimmyJames: @Ewan Using exceptions for control flow has been considered acceptable in Python development for quite some time.
• JimmyJames: @Ewan What's your python experience exactly? The top answer here explains why it's basically unavoidable. Please show some reference for your claim.

Answer 1

This is a somewhat subjective thing and I think you can make an argument for many different approaches here (including duplication) but here's one I might consider based on what you've shown.

def process(object):
 try:
 if suitedForA(object):
 methodA(object)
 return
 except SomeExceptionType:
 pass # log if desireable
 try:
 methodB(object)
 except SomeExceptionType:
 methodC(object)

As a sidenote, I highly recommend you start following the PEP 8 standards. I have found pylama to be a good tool for helping with that.

Answer 2

Slightly more verbose than the accepted answer and functionally the same, but I prefer writing it like this:

def tryMethodA(object):
 try:
 methodA(object)
 return True
 except SomeException:
 return False
def tryMethodB(object):
 try:
 methodB(object)
 return True
 except SomeException:
 return False
 
def process(object):
 if suitedForA(object) and tryMethodA(object):
 return
 
 if tryMethodB(object):
 return
 
 methodC(object)

To me, this more clearly communicates that methodA and methodB are unsafe or expected to fail, and that all three methods are equally viable but with different priorities. It removes a level of nested indentation and makes process more succinct at the cost of two new wrapper methods. However, you can then put additional exception handling logic like logging or a retry mechanism in the tryMethod methods without cluttering process.

• JimmyJames: There are definitely advantages to this. If you put the 'issuited' check within the methodA wrapper, you could then do things like put the method references in a sequence and loop over it until one returns True. That's useful if you have a long set of methods and/or need to make things configurable.

Answer 3

Depending on what methods A and B are doing, you might be able to get away with skipping the suitedForA call and simply attempt each one in order. This assumes by calling methodA when ill-suited, that you'll get an exception. It's a big assumption, but this allows another option to avoid duplication.

# if the methods are cheap and easy
def process(object):
 try:
 methodA(object)
 except:
 try:
 methodB(object)
 except:
 methodC(object)

The problem with the above is in cases where no exception is raised. object was suited for B but instead we ran the the incorrect method.

So from a safety standpoint it might be good idea to check for the condition that tells you which method to call. This assumes you have that capability to distinguish between the three objects ahead of time.

def process(object):
 if suitedForA(object):
 methodA(object)
 elif suitedForB(object):
 methodB(object)
 elif suitedForC(object):
 methodC(object)
 else:
 raise SomeException("We dont know what to do here")

If you own the objects, it's worth considering a SOLID approach and let them dictate what is the appropriate method to run.

class ObjectA:
 def process(self):
 return methodA(self)
class ObjectB:
 def process(self):
 return methodB(self)
class ObjectC:
 def process(self):
 return methodC(self)
def process(object):
 object.process()

---

Lastly, sometimes WET code is ok. DRY is a goal, but it isn't the only goal.

• JimmyJames: This seems to lose the fallback behavior that was inherent in the question.
• Marcel Wilson: @JimmyJames I agree. Without the context of what the sample code actually does, it's hard to know. I would posit that fallback isn't the only solution.
• JimmyJames: I thought it was pretty clear that the OP was trying to avoid duplication as a result of the need to call B in the both the case that A isn't applicable and if A failed. The call to C is only shown as a fallback. In general, you can't protect against exceptions in a pre-check. For example, an IO error must be handled at the time of the call.
• Marcel Wilson: That is very likely the case. I removed the assumption that it was necessary in this solution since yours already covered that.

Answer 4

I'm not a native Python programmer, so this is going to be a bit of a creole, but I'd suggest something like this:

def process(object)
 bool suitsA = suitedForA(object)
 bool successA;
 bool successB;
 bool successC;
 //If it suits A, then try A
 if suitsA:
 try:
 methodA(object)
 successA = true
 except:
 successA = false
 //If it does not suit A, or if A failed
 if not suitsA or not successA:
 try:
 methodB(object)
 successB = true
 except:
 successB = false 
 //If neither A nor B succeeded
 if not successA and not successB:
 try:
 methodC(object)
 successC = true
 except:
 successC = false
 //If nothing worked
 if not succeessA and not successB and not successC:
 raise FailureException()

It's a little more long-winded than the original certainly, but undoubtedly the intention is clearer and the structure more regular, and there is no repetition.