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?

32
  • 7
    dont use exceptions for flow control
    – Ewan
    Commented Dec 13, 2022 at 14:01
  • 2
    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):?
    – Kevin
    Commented Dec 13, 2022 at 14:57
  • 1
    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. Commented Dec 13, 2022 at 15:36
  • 1
    @Ewan Using exceptions for control flow has been considered acceptable in Python development for quite some time.
    – JimmyJames
    Commented Dec 13, 2022 at 16:52
  • 1
    @Ewan What's your python experience exactly? The top answer here explains why it's basically unavoidable. Please show some reference for your claim.
    – JimmyJames
    Commented Dec 13, 2022 at 18:35

4 Answers 4

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.

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.

1
  • 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.
    – JimmyJames
    Commented Dec 22, 2022 at 17:40
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.

4
  • This seems to lose the fallback behavior that was inherent in the question.
    – JimmyJames
    Commented Dec 22, 2022 at 17:42
  • @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. Commented Dec 22, 2022 at 18:04
  • 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.
    – JimmyJames
    Commented Dec 22, 2022 at 19:12
  • That is very likely the case. I removed the assumption that it was necessary in this solution since yours already covered that. Commented Dec 22, 2022 at 19:37
-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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.