Whether to check for null

Question

I know that you should always check incoming params to a method for null. But what if I have this scenario with a try/catch referring to a local variable. Do I really need to check for null below? Because it's gonna catch it anyway if it's null and the next line of code tries to use the refundResponse variable:

    public string DoRefund(...)
    {
        try
        {
    ......
            string refundTransactionID = string.Empty;
    ......

            RefundTransactionResponseType refundResponse = transaction.DoRefund(...);

            if (refundResponse != null)
                refundTransactionID = refundResponse.RefundTransactionID;
    .....
        }
        catch (Exception ex)
        {
            LogError(ex);
            return ex.ToString();
        }
    }

Remember I'm talking specifically about local variables and checking those inside a method, not incoming params to a method.

All I'm asking here is do I need to check for null before setting refundTransactionID or do I just set it without the if assuming that the compiler will handle and throw if it is null which will be caught and thrown back as a string to the caller in this case.

or should it be

if (refundResponse == null)
                return null;

or just take the check out completely for this local variable assignment and then since in this case I have a try/catch I'm handling any exceptions picked up by the compiler naturally by returning the exception as a string to the caller (it was not my decision to send back a string, it was a requirement by my boss...so bypass that debate for now):

 refundTransactionID = refundResponse.RefundTransactionID;

ultimately the rest of the code further down the line in the method is dependent on a valid refundTransactionID.

Answer 1

Exceptions are for exceptional conditions. If you can check for a continuable error, do so, please!

Answer 2

No you don't need to check for null, there. That opens up another question, though, do you really need to check for null in incoming parameters?

Remember: that's a behavior. You have to test that behavior.

Answer 3

I'd suggest checking for the null then doing some kind of soft error handling instead of just letting it catch and throwing an error message.

Answer 4

But if you can't continue at that point let the exception propogate.

Answer 5

I know that you should always check incoming params to a method for null.

No, not necessarily. What you should specify is the contract of your method. It's perfectly acceptable (and common) to specify that you'll throw a NullPointer/NullReferenceException for a null parameter. Then you don't need any checking.

You can also check for null, but this only makes sense if you can actually handle a null usefully (e.g. substitute a default value).

Answer 6

You should have to check for null in that instance. Your application logic should be able to handle these kind of situations, without the need for exceptions.

Answer 7

No, doesn't look like you should check for null here. And I also wouldn't check for null for ALL incoming parameters (as your description suggests).

It's also odd that you're returning a transactionID as a string OR the message of an exception. How will the caller of this method know if an exception happened?

If you really want to log the exception, how about something like this:

    public string DoRefund(...) 
    { 
        try 
        {
            return transaction.DoRefund(...).RefundTransactionID; 
        } 
        catch (Exception ex) 
        { 
            LogError(ex); 
            throw ex;
        } 
    }

Answer 8

An alternative to testing is the Null Object pattern. Instead of returning Null, or a valid transaction, the transaction::DoRefund() method returns a null object: an object that offers the same interface as the RefundTransactionResponseType instances, but its methods do nothing. With this there is no need to test whether for Null.

The should be used wisely as this can easily hide problems.

Answer 9

You should check for null rather than letting the exception handling handle it. As leppie said, exceptions are for exceptional conditions not normal flow of control. If you know what issues can occur then you should gracefully handle them.

Another thing to keep in mind is the performance impact of exceptions. When the exception is thrown the JVM has to unwind the call stack. In your example the exception is then also logged. All of this takes time and is much slower than a simple "if" check.

Answer 10

It depends on what it means to your program when (refundResponse == null). If this has some meaning, then it makes sense to report a more informative error. If it should never happen and would indicate a flaw in the DoRefund method, then I think it's fine to allow the null to cause an exception later. In the latter case, I'd only have a specific check if you're suspicious of the method and whether it's behaving as it's supposed to.