3

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?

4 Answers4

10

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.

JimmyJames supports Canada
  • 30,578
  • 3
  • 59
  • 108
4

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.

0

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.

-1

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.

Steve
  • 12,325
  • 2
  • 19
  • 35