7

We currently have the following compound if statement...

if ((billingRemoteService == null)
    || billingRemoteService.getServiceHeader() == null
    || !"00".equals(billingRemoteService.getServiceHeader().getStatusCode())
    || (billingRemoteService.getServiceBody() == null) 
    || (billingRemoteService.getServiceBody().getServiceResponse() == null) 
    || (billingRemoteService.getServiceBody().getServiceResponse().getCustomersList() == null) 
    || (billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList() == null) 
    || (billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList().get(0) == null) 
    || (billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList().get(0).getBillAccountInfo() == null)
    || (billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList().get(0).getBillAccountInfo().getEcpdId() == null)) {
        throw new WebservicesException("Failed to get information for Account Number " + accountNo);
}

return billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList().get(0);

Couldn't this be simplified as ...

try {
    //Check to be sure there is an EpcdId.
    (billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList().get(0).getBillAccountInfo().getEcpdId();
    return billingRemoteService.getServiceBody().getServiceResponse().getCustomersList().getCustomersList().get(0);
} catch (NullPointerException npe) {
    throw new WebservicesException("Failed to get information for Account Number " + accountNo);
}

If so, what is the "cost" difference between the two approaches under Java 6? It seems like a pretty complex if statement just to verify that all of the intervening calls aren't null. This operation is called many times for different accounts.

tshepang
  • 12,111
  • 21
  • 91
  • 136
David Nedrow
  • 1,098
  • 1
  • 9
  • 26
  • 9
    Performance aside, the second snippet is very ugly. Unchecked exceptions should be prevented not caught. This is not what exceptions are meant for. – Tudor Sep 10 '12 at 20:09
  • 4
    Other people would find the second snippet more appealing. I honestly don't see why not catching unchecked exceptions in such a case. – E_net4 Sep 10 '12 at 20:11
  • Aye, agreed. Yet much of the other code is "untouchable". I'm stuck with what I have and just wondered if the if() statement could be simplified without increasing the operational cost. – David Nedrow Sep 10 '12 at 20:12
  • Have a look here: http://stackoverflow.com/questions/299068/how-slow-are-java-exceptions – Tudor Sep 10 '12 at 20:13
  • you are breaking law of Demeter. I would highly recommend you go over your design again. – DarthVader Sep 10 '12 at 20:29
  • @DarthVader: I'm afraid that this is some sort of autogenerated webservice client code which the OP don't have control over. They can indeed get very hairy like that. – BalusC Sep 10 '12 at 20:39
  • 1
    @BalusC Even auto-generated code can be wrapped by a stationary interface. Put a Facade on that beast. – Edwin Buck Sep 11 '12 at 13:51

7 Answers7

7

I must disagree with Edwin Buck's argument.

He says:

As others have stated, exceptions are more expensive than if statements. However, there is an excellent reason not to use them in your case. "Exceptions are for exceptional events"

When unpacking a message, something not being in the message is expected error checking, not an exceptional event.

This is essentially saying that if you do error checking then an error is expected (because you were looking for it) and therefore not exceptional.

But that is not what "exceptional event" means. An exceptional event means an unusual / out of the ordinary / unlikely event. Exceptional is about the likelihood of the event happening, not about whether you are (or should be) expecting and/or looking for it.

So going back to first principles, the underlying reasoning for avoiding exceptions is a cost trade-off: the cost of explicitly testing for the event versus the cost of throwing, catching and handling the exception. To be precise.

If the probability of the event is P

  • the average cost of using exceptions is:

    P * cost of an exception being created/thrown/caught/handled + (1 - P) * cost of no explicit tests

  • the average cost of not using exceptions is:

    P * cost testing for the condition when it occurs and doing the error handling + (1 - P) * cost of testing when the condition doesn't occur.

And of course, this is where "exceptional" == "unlikely" comes in. Because, if as P gets closer to 0, the overheads of using exceptions become less and less significant. And if P is sufficiently small (depending on the problem), exceptions will be MORE efficient.


So in answer to the original question, it is not simply the cost of if / else versus exceptions. You also need to take account of the likelihood of the event (error) that you are testing for.

The other thing to note is that there is a lot of scope for the JIT compiler to optimize both versions.

  • In the first version, there is potentially a lot repeated calculation of subexpressions, and repeated behind-the-scenes null checking. The JIT compiler may be able to optimize some of this, though it depends whether there might side-effects. If it can't, then the sequence of tests could be rather expensive.

  • In the second version, there is scope for the JIT compiler to notice that an exception is being thrown and caught in the same method without making use of the exception object. Since the exception object doesn't "escape" it could (in theory) be optimized away. And if that happen, the overheads of using exceptions will almost vanish.


(Here is a worked example to make it clear what my informal equations mean:

  // Version 1
  if (someTest()) {
      doIt();
  } else {
      recover();
  }

  // Version 2
  try {
      doIt();
  } catch (SomeException ex) {
      recover();
  }

As before, let P be the probability that the exception-causing even occurs.

Version #1 - if we assume that the cost of someTest() is the same whether the test succeeds of fail, and use "doIt-success" to denote the cost of doIt when no exception is thrown, then the average cost of one execution of version #1 is:

  V1 = cost("someTest") + P * cost("recover") + (1 - P) * cost("doIt-success")

Version #2 - if we assume that the cost of doIt() is the same whether or not an exception is thrown, then the average cost of one execution of version #2 is:

  v2 = P * ( cost("doit-fail") + cost("throw/catch") + cost("recover") ) +
       (1 - P) * cost("doIt-success")

We subtract one from the other to give the difference in average costs.

  V1 - V2 = cost("someTest") + P * cost("recover") + 
            (1 - P) * cost("doIt-success") -
            P * cost("doit-fail") - P * cost("throw/catch") -
            P * cost("recover") - (1 - P) * cost("doIt-success")

          = cost("someTest") - P * ( cost("doit-fail") + cost("throw/catch") )

Notice that the costs of recover() and the costs where doIt() succeed cancel out. We are left with a positive component (the cost doing the test to avoid the exception) and a negative component that is proportional to probability of the failure. The equation tells us that no matter how expensive the throw / catch overheads are, if the probability P is close enough to zero, the difference will be negative.


In response to this comment:

The real reason you shouldn't catch unchecked exceptions for flow control is this: what happens if one of the methods you call throws an NPE? You catching the NPE assumes that it came from your code when it may have come from one of the getters. You may be hiding a bug underneath the code and this can lead to massive debugging headaches (personal experience). Performance arguments are useless when you might be hiding bugs in your (or others') code by catching an unchecked exception like NPE or IOOBE for flow control.

This is really the same argument as Edwin Bucks's.

The problem is what does "flow control" mean?

  • On the one hand, throwing and catching exceptions is a form of flow control. So that would mean that you should never throw and catch unchecked exceptions. That clearly makes no sense.

  • So then we fall back to arguing about different kinds of flow control, which is really the same arguing about what is "exceptional" versus what it "non-exceptional".

I recognize that you need to be careful when catching NPE's and similar to make sure that you don't catch one that comes from an unexpected source (i.e. a different bug). But in the OP's example, there is minimal risk of that. And you can and should check that those things that look like simple getters really are simple getters.

And you also have to recognize that catching NPE (in this case) results in simpler code, that is likely to be more reliable than a long sequence of conditions in an if statement. Bear in mind that this "pattern" could be replicated in lots of places.

The bottom line is that the choice between exceptions and tests CAN BE a complicated. A simple mantra which tells you to always use tests is going to give you the wrong solution in some situations. And the "wrongness" could be less reliable and/or less readable and/or slower code.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • I think you are confusing probablity with cost, or more specifically amortized cost (the cost of all exceptions in the program over it's execution) with cost (the cost of one exception). If an exception takes 20 times the cost of an if statement (probably not completely true) then throwing fewer exceptions might make the program faster, but it doesn't actually reduce the cost of any single exception. Exceptions should be exceptional because they break the flow of the natural execution of a block of code, and you don't want most of your processing done outside of the code flow. – Edwin Buck Sep 13 '12 at 12:47
  • No I am not doing that. If you read my "equations" you will see that I multiply the event probabilities by the event costs to give the average (i.e. amortized) cost. – Stephen C Sep 13 '12 at 13:44
  • *"Exceptions should be exceptional because they break the flow of the natural execution of a block of code ..."* This is another circular definition, that doesn't really address the issue of whether you should use exceptions. The issue is not whether the exceptions are exceptional. It is whether the EVENTS (that you detect and then either throw or don't throw an exception for) are exceptional. – Stephen C Sep 13 '12 at 13:51
  • It is not a circular definition. I didn't say an exception is an exception. I said that when analyzing a block of code for it's purpose, one shouldn't make the main route of processing go through the out-of-flow exception processing blocks. Recovery in those blocks should be exactly that, recovery. It shouldn't be part of normal processing, as few exception blocks are properly written to handle exceptions within the exception block, and too many layers of exception handling hinder readability, which is important in the isolation and discovery of bugs. – Edwin Buck Sep 13 '12 at 15:20
  • *"I said that when analyzing a block of code for it's purpose, one shouldn't make the main route of processing go through the out-of-flow exception processing blocks."*. I agree with THAT. Indeed, if 'P' is small, exceptions WON'T be the main route of processing. On the topic of readability, it depends on the readability of the alternative. For instance, how much code is dedicated to tests to prevent exceptions, to check "status" values returned by methods and so on. And whether *that* code is robust and readable. The OP's code is an example where exceptions are certainly more readable. – Stephen C Sep 14 '12 at 10:11
  • Sorry to reopen old discussions, but this whole argument for time is moot. The real reason you shouldn't catch unchecked exceptions for flow control is this: what happens if one of the methods you _call_ throws an NPE? You catching the NPE assumes that it came from your code when it may have come from one of the getters. You may be hiding a bug underneath the code and this can lead to massive debugging headaches (personal experience). Performance arguments are useless when you might be hiding bugs in your (or others') code by catching an unchecked exception like NPE or IOOBE for flow control. – Brian Sep 26 '12 at 02:36
  • @Brian - Maybe. It could moot in this example *if those exceptions are possible*. However, we can posit that *sections* of code can be examined and *proven* to be free of NPE's (or other specified exceptions). Also that whole scenario of lots of different things that could be `null` speaks (to me) of poor design. There should be earlier checks that tested for the "missing" elements and threw a custom exception (not NPE). – Stephen C May 06 '13 at 22:57
  • @Brian - The other point is that while this is a bad example, the points I am arguing remains true. 1) The performance for exceptions and exception handling is NOT black and white. 2) The "no exceptions for flow control" and "exceptions are for exceptional events" mantras have a definite "smell" of circular logic about them. – Stephen C May 06 '13 at 23:01
2

As others have stated, exceptions are more expensive than if statements. However, there is an excellent reason not to use them in your case.

Exceptions are for exceptional events

When unpacking a message, something not being in the message is expected error checking, not an exceptional event.

This block of code is far too interested in the data in other instances. Add some behavior to those other instances. Right now all of the behavior is in the code that is not in the class, which is bad object orientation.

-- for billingRemoteService --
public boolean hasResponse();
public BillingRemoteResponse getResponse();

-- for BillingRemoteResponse --
public List<Customer> getCustomerList();

-- for Customer --
public Customer(Long ecpdId, ...) {
  if (ecpdId == null) throw new IllegalArgumentException(...);
}
Edwin Buck
  • 69,361
  • 7
  • 100
  • 138
1

You could mix Groovy into your JVM-based application - the line will get considerably simpler then:

def result = billingRemoteService?.
  serviceBody?.
  serviceResponse?.
  customersList?.
  customersList[0];
if ('00' != billingRemoteService?.serviceHeader?.statusCode ||
   result?.
   billAccountInfo?.
   getEcpdId == null)
  throw new WebServicesException
...
nd.
  • 8,699
  • 2
  • 32
  • 42
  • 1
    One of the COIN proposals was to add an "Elvis" operator to Java for Java 7. It didn't make the cut ... and hasn't been seen since. – Stephen C Mar 11 '13 at 09:39
1

You can avoid both try/catch and compound ifs when using Java 8 optionals:

import java.util.List;
import java.util.Optional;

public class Optionals
{
    interface BillingRemoteService
    {Optional<ServiceBody> getServiceBody();}

    interface ServiceBody
    {Optional<ServiceResponse> getServiceResponse();}

    interface ServiceResponse
    {Optional<List<Customer>> getCustomersList();}

    interface Customer
    {Optional<BillAccountInfo> getBillAccountInfo();}

    interface BillAccountInfo
    {Optional<EcpId> getEcpdId();}

    interface EcpId
    {Optional<BillingRemoteService> getEcpdId();}

    Object test(BillingRemoteService billingRemoteService) throws Exception
    {
        return
        billingRemoteService.getServiceBody()
        .flatMap(ServiceBody::getServiceResponse)
        .flatMap(ServiceResponse::getCustomersList)
        .map(l->l.get(0))
        .flatMap(Optional::ofNullable)
        .flatMap(Customer::getBillAccountInfo)
        .flatMap(BillAccountInfo::getEcpdId).orElseThrow(()->new Exception("Failed to get information for Account Number "));
    }
}

This requires you to change the signature of those methods but if they often return null this should be done anyways in my opinion. If null is not expected but an exception, and changing the method signature is not possible then exceptions can be used. I don't think the runtime difference is a problem unless you call this operation hundreds of thousands of times per second.

Konrad Höffner
  • 11,100
  • 16
  • 60
  • 118
0

As a rule of thumb exception handling is more expensive than ifs, but I agree with TheZ that the best approach would be to benchmark / profile both versions under the expected load. The difference may become negligible when you consider IO and networking costs which generally extrapolates CPU costs by orders of magnitude. Also, please notice that the !00.equals condition should probably be checked in the second version.

Anthony Accioly
  • 21,918
  • 9
  • 70
  • 118
0

essentially if/else vs try catch are not same things. Performance/cost of these things are irrelevant for your code. Once you design and implement your business rules correctly then you can worry about performance.

try/catch is for exception handling for exceptional states of the program. while if else is for conditional states of the program.

They have their own uses. See what you need and code accordingly.

Moreover, you are breaking law of demeter. it would be better if you go over design and code for making it simpler to read as well as make it a better design for maintainability and extensibility.

DarthVader
  • 52,984
  • 76
  • 209
  • 300
0

OK, none of the answers really answered the question, though theZ's suggestion was the fastest way to check this given my current circumstances. None of this code was designed or written by me, and the application it is part of is massive, which would mean person-years of refactoring to handle every situation like this.

So, for everyone's edification:

I whipped up a quick test that mocks the classes required for both methods. I don't care how long any of the individual methods of the classes run, as it is irrelevant to my question. I also built/ran with JDKs 1.6 and 1.7. There was virtually no difference between the two JDKs.

If things work --- IE no nulls anywhere, the average times are:

Method A (compound IF):  4ms
Method B (exceptions):   2ms

So using exceptions when objects are not null is twice as fast as a compound IF.

Things get even more interesting if I deliberately force a null pointer exception at the get(0) statement.

The averages here are:

Method A: 36ms
Method B:  6ms

So, it's clear that in the original case documented, exceptions are the way to go, cost-wise.

David Nedrow
  • 1,098
  • 1
  • 9
  • 26
  • 2
    Your results are **highly suspicious**. They seem to be saying that when one of the explicit null tests gives `true` (which short-circuits the remaining tests), the time jumps from 4ms to 36ms. This is just so counter-intuitive that it *has to be* nonsense; i.e. there *has to be* something wrong with your benchmarking methodology. Please post the code. – Stephen C Sep 11 '12 at 02:44
  • 1
    The most likely explanation is that the 4ms and 2ms are before the code was JIT compiled, and the 6ms is after. And that the 36ms is including the time taken to JIT compile the benchmark! – Stephen C Sep 11 '12 at 02:47
  • Stephen C is correct, my results were bad. After making a simple change, the average times are now 51ms for Method A, and 48ms for Method B. Exceptions are still slightly faster than the compound if. Given how many of these calls are made, I'm still going to go for an exception. – David Nedrow Sep 11 '12 at 05:50
  • Exceptions being slightly faster is plausible if consider what the JIT optimizer might or might not be able to do; see my Answer. However, I'd still like to see your complete benchmark code to figure out what is going on. And I should note that your result only applies to your specific use-case. In many others, you will find that exceptions **are** significantly slower. – Stephen C Sep 11 '12 at 06:24