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?

Jann Poppinga
  • 159
  • 4
  • 7
    dont use exceptions for flow control – Ewan Dec 13 '22 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 Dec 13 '22 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. – Greg Burghardt Dec 13 '22 at 15:36
  • 1
    @Ewan Using exceptions for control flow has been considered acceptable in Python development for quite some time. – JimmyJames Dec 13 '22 at 16:52
  • @Ewan: using exceptions for flow control is surely a code smell, but code not being DRY also. But far worse than any code smell is dogmatism. – Doc Brown Dec 13 '22 at 17:34
  • @JimmyJames I think your out of date there, but also its still wrong. – Ewan Dec 13 '22 at 17:38
  • 1
    @Ewan What's your python experience exactly? The top answer [here](https://stackoverflow.com/questions/16138232/is-it-a-good-practice-to-use-try-except-else-in-python#:~:text=In%20many%20cases%2C%20you%20must%20use%20exceptions%20for,of%20whether%20you%20actually%20use%20exceptions%20or%20not%29.) explains why it's basically unavoidable. Please show some reference for your claim. – JimmyJames Dec 13 '22 at 18:35
  • 1
    https://softwareengineering.stackexchange.com/questions/351110/are-exceptions-for-flow-control-best-practice-in-python – Ewan Dec 13 '22 at 18:47
  • My view is that exception control flow is used in python because the code has been developed by mathematicians rather than "professional programmers" its easy to say if A fails then run B and there's no massive cost to it. But examples like this show why its not good practice (outside of iterators and 'it costs loads to check and redo if ok' scenarios). In this case you cant say what the optimal case is for sure, but if you can check `suitedForA` then check `suitedForB/C/D` etc and just error on exceptions. – Ewan Dec 13 '22 at 18:51
  • @Ewan Did you read the answers? Look at amon's: "The general consensus “don't use exceptions!” mostly comes from other languages and even there is sometimes outdated" – JimmyJames Dec 13 '22 at 18:57
  • @ewan If you think [Guido van Rossum](https://en.wikipedia.org/wiki/Guido_van_Rossum) (currently a 'Distinguished Engineer' at MS) is not a 'professional programmer' you are incredibly confused. – JimmyJames Dec 13 '22 at 19:01
  • ok well if we are just name checking rather than actually talking about whats best and why im out – Ewan Dec 13 '22 at 19:03
  • It just seems like you are making a lot of unsubstantiated claims. You realize Guido created the language, right? You claimed that the people who developed it are not 'professional programmers'. Where did you get the idea that they were mathematicians? – JimmyJames Dec 13 '22 at 19:06
  • @ewan `suitedForA()` is independent of whether `methodA()` will fail. `methodA/B/C()` are library functions that sometimes fail without me being able to tell when. – Jann Poppinga Dec 14 '22 at 07:19
  • It really looks to me as if methodB can be called twice. I’d first want to know what exactly needs to happen. I’d probably have some booleans telling me what still needs calling. – gnasher729 Dec 14 '22 at 21:23
  • @gnasher729 Yes, thanks, I removed the error. The three methods act as consecutive fallback in case of error, with the caveat that sometimes `methodA()` cannot be used. – Jann Poppinga Dec 15 '22 at 10:15
  • 1
    Since your actual scenario involves unknown library code I don't know if this is reasonable or not, but in my opinion an item that is `suitedForA` should never throw an *expected* exception when passed into `methodA`. It may be an issue of verbiage more than anything else, but I would not expect a `disc` object that passes `isDvd` but throws an exception in `readDvd` to then be an eligible candidate for `readBluRay`, just to give an example. – Michael Brandon Morris Dec 20 '22 at 01:47
  • 1
    @MichaelBrandonMorris It's more like: If you're afraid of flying, you pass on the helicopter, but if not, it might still not take off. In both cases, you fall back on the boat. – Jann Poppinga Dec 20 '22 at 20:22
  • Does `suitedForA` represent something like `suitedForFlying` or more like `suitedForRotarWing` ? – Marcel Wilson Dec 22 '22 at 19:59
  • @JimmyJames I'm with @Ewan! I think Python does it wrong. :-D It's in the name, "exception"; it's supposed to be a very different and abnormal type of control flow resulting from something wholly unexpected, not something that should need to be handled in every function that wants to test if a key exists in an associative structure, e.g. That LBYL Python philosophy is so counter-productive IMO to practical exception handling. If even 25% of our functions need to handle thrown exceptions, then I think we've largely lost the benefits of exception handling over plain old C-style error handling. – Anti Gamer Jan 31 '23 at 01:35
  • 1
    @AntiGamer You are free to have your opinion, of course. But telling experienced Python developers that they are 'doing it wrong' is unlikely to impress them or change any minds. The Python community, as a whole, just doesn't care about that particular orthodoxy. – JimmyJames Jan 31 '23 at 16:07
  • 1
    @JimmyJames Cheers! I think, coming from C-style error handling to exception handling, the main benefit of exception handling is when the depth from caller to callee that throws can be quite deep, automating away the need to propagate and handle errors manually and roll back transactions for all intermediate levels of the call stack. That can be extremely time-saving and automates away a lot of potential for human error. Yet I see little benefit in regular control flow cases where the depth between caller and thrower is almost always one, like checking to see if a key exists in a map. – Anti Gamer Jan 31 '23 at 20:04
  • 1
    @JimmyJames ... in those cases, I think we end up with a lot of extra boilerplate writing try and catch code for what would normally be regular, not exceptional, control flow. But beyond the boilerplate and gruntwork, the bigger concern I have is that using exceptions for such regular control flow will cause them to be jumbled up with truly exceptional cases, like a memory allocation failure or a server disconnect in the middle of an operation. It might tempt devs to swallow up and fail to handle the truly exceptional cases if code is throwing for non-exceptional cases by catching generally. – Anti Gamer Jan 31 '23 at 20:07
  • 1
    @AntiGamer I'm a (recovering) Java developer and I am familiar with how they are 'supposed' to be used in similar languages. This aspect of Python was alien to me at first but then I came to accept it. Some of the uses of exceptions such as a dict (map) lookup can be replaced with preferred solutions in most cases. That is, use .get(key, default) instead of catching KeyError. But if that doesn't work (or using a defaultdict) then I don't really see much issue after nearly two decades of using Python. It's just not that big of a deal. It's easy enough to follow. – JimmyJames Jan 31 '23 at 22:20
  • 1
    @AntiGamer I do acknowledge that there's a risk if you catch too broadly. That's one of the reasons to follow PEP-8 standards and always declare the type of exception being caught. If you are using this pattern for missing keys in a dict, you should always catch like so: `except KeyError` – JimmyJames Jan 31 '23 at 22:23
  • @JimmyJames I see! I'm mostly coming from a C++ mindset. I find the productivity of exceptions from the side of the catcher to be maximized when we can catch as generally as possible (ex: `std::exception`). I'm down for discussing different philosophies and outlooks though! It's just, at least from my perspective and there could very well be a bias there, I think it degrades the productivity of exceptions to have to write a lot of exception-handling code catching things very specifically that often fit into regular and expected control flow like `KeyError`. – Anti Gamer Jan 31 '23 at 22:23
  • @JimmyJames Admittedly I'm quite deeply-opinionated on this topic, as I was the one on my team that had to persuade a lot of old school C programmers to embrace exception handling despite their suspicions of it, and its overall performance, when we started migrating (very slowly) to C++ in the late 90s. So I might have an overly argumentative side to me when it comes to exception handling and how I believe it can streamline productivity; apologies for that! – Anti Gamer Jan 31 '23 at 22:26
  • 1
    @AntiGamer Around 2004 I decided to learn (and embrace) Python because I was constantly debating about and claiming the superiority of static typing but realized I had no leg to stand on. I had no experience with the alternative. At the time Ruby was all the rage but someone suggested Python instead. I wish I remember who it was (clearly, I respected their opinion) because I found it to be one of the most rewarding things I've undertaken as a programmer. Not only because Python eventually came out on top but it also really transformed the way I think about programming. – JimmyJames Jan 31 '23 at 22:32
  • @JimmyJames I hope it's okay to be extending the comments so long. I'll try to stop here but static typing is something I lean on so heavily. I've had a lot of difficulty with dynamically-typed languages like Lua and Python since, it might not be so good, but I lean heavily on the compiler to point out all the places that need updating in code when I'm required to change a public interface, e.g., without any possibility of missing an obscure place because the detection of the type change can only happen when running the software. It's at least a great difficulty for me to not have that. – Anti Gamer Jan 31 '23 at 22:34
  • 1
    @JimmyJames I suspect though that this could be solved with much more comprehensive unit testing, allowing the unit tests to detect those errors that result from changes rather than the compiler. It's just something I'm not used to and so comfortable with since, while I absolutely value writing as many tests as possible and TDD, I still rely on the compiler so heavily to point things out at compile-time as much as possible. – Anti Gamer Jan 31 '23 at 22:37
  • 1
    @AntiGamer I think we'll be OK here with the extended discussion. I'll just say that it's "horses for courses". There are a lot of problems that don't require strictness. Many packages in Python are written in C for good reason. It's a good control language; originally one of the main uses was to replace shell scripts. – JimmyJames Jan 31 '23 at 22:44
  • @MarcelWilson Doesn't matter in this context because helicopter is the only method of air travel available. – Jann Poppinga Feb 06 '23 at 12:54

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
  • 24,682
  • 2
  • 50
  • 92
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.

Michael Brandon Morris
  • 209
  • 1
  • 8
  • 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 Dec 22 '22 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.

Marcel Wilson
  • 113
  • 7
  • This seems to lose the fallback behavior that was inherent in the question. – JimmyJames Dec 22 '22 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. – Marcel Wilson Dec 22 '22 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 Dec 22 '22 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. – Marcel Wilson Dec 22 '22 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.

Steve
  • 6,998
  • 1
  • 14
  • 24