Question

I've read Defensive Programming vs Exception Handling? and if/else statements or exceptions, but none contain something relevant to what I'm searching for.

Taking into account that exception handling is more computational expensive than just carefully designing your code and avoiding certain scenarios, I recently came to the following question: what is the computational complexity "threshold" over which Exception Handling should be preferred?

In my case, I am receiving data from an external middleware and some times fields are null. There are two cases I can handle this.

First

try {
    if (user.departNameEn.Equals(selectedUser) 
    || user.departNameFr.Equals(selectedUser) 
    || user.departNameDe.Equals(selectedUser)) {
        string fileName = AppDomain.CurrentDomain.BaseDirectory 
            + @"Images\Users\" + user.Id + user.ShortName + ".png";
        if (File.Exists(fileName))
        {
            retVal = fileName;
            return retVal;
        }
    }
}
catch (NullReferenceException e)
{
    continue;
}

Second

if (!string.IsNullOrEmpty(user.departNameEn)
    && !string.IsNullOrEmpty(user.departNameFr)
    && !string.IsNullOrEmpty(user.departNameDe)
    && !string.IsNullOrEmpty(user.Id)
    && !string.IsNullOrEmpty(user.ShortName)) {
    if (user.departNameEn.Equals(selectedUser) 
    || user.departNameFr.Equals(selectedUser) 
    || user.departNameDe.Equals(selectedUser)) {
        string fileName = AppDomain.CurrentDomain.BaseDirectory 
            + @"Images\Users\" + user.Id + user.ShortName + ".png";
        if (File.Exists(fileName))
        {
            retVal = fileName;
            return retVal;
        }
    }
}

Intuitively, the first approach is much more clean and elegant than the second one but which of these is computationally less expensive? Where do the limits of defensive programming end?

Note to duplicate report: As I explained in the introduction, I've read the two main topics for Exception Handling vs Defensive Programming but what I'm actually asking is something different and very specific: when Defensive Programming becomes computationally more expensive than just simplifying your code and adding a single try-catch statement?

Was it helpful?

Solution

I am receiving data from an external middleware

File.Exists

So let's look at the bigger picture. You have data. That data was serialized. Then send over a wire. Then deserialized. And in the end, you even access the file system based on that data.

Looking at how you handle this from a perspective of performance is completely unnecessary, because computation power is really not the bottleneck by far in your scenario. It's like wondering whether the diver of an 18-wheeler should be put on a diet to save on overall weight.

As for the 18 wheeler: the best solution is to let the driver do whatever makes him happy and motivated to drive their truck. A pound or two in either direction really do not matter. So do what is best for the programmer. Whatever is the more readable, easier to understand code by your standards is the best solution here.


On an unrelated note:

user.departNameEn.Equals(selectedUser)

is there a reason you call Equals or is that maybe just a bad Java habit?

user?.departNameEn == selectedUser

This line does the same and will not throw on one side being null. The only "disadvantage" would be that if selectedUser is null it's considered equal too, instead of throwing as in your line.

OTHER TIPS

Are you expecting null data?

Exceptions are intended for exceptional circumstances.

The simple rule of thumb for situations like this is to handle standard cases with logic, not exceptions.
If a NULL is an error, indicating a transmission failure for example, then an exception is suitable. If however, it is indicating that an optional field is empty, then this is a "normal" use-case, and should be checked with logic.

The example used in the question implies that the data in invalid if some data is missing (and that this is unexpected).
I would recommend exceptions in this case.

This depends on more information than you are giving us. What does it mean if some fields are null? Is that (1) totally admissible and should just be interpreted as a value of ""? (2) not supposed to happen but common in practice and susceptible to a workaround? (3) possible to deal with but only with user input? (4) a flaw that makes processing that record impossible? (5) a sign of such serious trouble that the entire import must be abandoned ("halt and catch fire")?

Exceptions are not just more expensive to process at runtime, they also structure control flow in a different way than standard control structures. You can build almost any possible logical flow with if statements, but once you throw an exception out of a block there is no reasonable way of getting back into it again, and nested catch clauses are almost impossible to write in an readable way. Also, programming a try...catch raises certain expectations in readers of the code; if these are not actually true, this makes the code much harder to understand. Those are at least three different aspects of choosing how to handle uncommon situations; others can apply depending on what exactly your code is processing.

The two code examples are not logically equivalent.

In the first case just one needs to be filled and satisfy the condition. In the second case, all three need to be non-empty.

This in my opinion is the big thing why you should go for the first version. You made the code more complex and ended up with something else then intended. Use the code the expresses the best what you want to do in the easiest to understand way.

The goal of defensive programming is to check that everything you want is in the right state before starting not the "I don't wanna exception way" of doing things.

In your case : some field can be empty by defintion. So no checking for null is just to avoid NRE is not defensive programming it is the normal way of handling it.

On my side defensive programming is mainly based on throwing (in java) IllegalStateException/IllegalArgumentException a lot (so I'm more in offensive programming as defined per Wikipedia).

Furthermore the first sample code you provide to illustrate what you want to ask is definitively wrong whatever way you want to go : cathing a NullReferenceException is a no go. By doing that, you may catch the NRE you want but if the code evolve, you may also catch unwanted NRE specially if later, you or someone else update the code.

Finally as an aside for the code you quoted the best move is just to switch the side of your test : selectedUser.equals(...) because you are probably sure at this point that selectedUser is neither null nor empty. This is not a defensive programming solution, just what you should do to have the clearest code and avoid unwanted errors.

Licensed under: CC-BY-SA with attribution
scroll top