To Throw or Not to Throw

Question

//
// To Throw
void PrintType(object obj)
{
    if(obj == null) 
    {
        throw new ArgumentNullException("obj")
    }
    Console.WriteLine(obj.GetType().Name);    
}

//
// Not to Throw
void PrintType(object obj)
{
    if(obj != null)
    {
        Console.WriteLine(obj.GetType().Name);
    }
}

What principle to keep?

Personally I prefer the first one its say developer-friendly(notified about each "anomaly"). The second one its say user-friendly(let user continue work even if "inside" not everything does right).

I think that is more complicated to find errors and bugs in the maintenance phase when you silently let the things to go on. If something goes wrong you are not notified at once, and sometimes have errors far away from the main error cause, and spend a lot of time to find it.

What do you think?

Answer 1

The second one is lethal. Failing silently is always the wrong thing to do. Suppose this were the banking system in the bank that holds your account. Would you like it if there was a problem paying in your salary and the system silently ignored it?

Answer 2

Consider using the Null Object pattern is very useful to not clutter your code with try-catch, null checks (or god forbid swallowed errors).

Answer 3

Throw.

Let the caller of a function determine if it is important enough to throw an exception to the user on a null value, but the function itself should throw because of the invalid argument.

Answer 4

In this particular example, giving nothing to a printer is like saying "print nothing", thus working as it should.

I do know this is an example, but it's just to clarify that this is relative.

If your code displays user-friendly messages on exceptions somehow, what difference does it make ? the first one would be both developer and user friendly.

Answer 5

I'd say that it depends on your (developer) preference. From the user perspective, he should never see an unhandled exception, but it does not mean you cannot use exceptions.

I prefer the first one, because I find null to be a totally unnecessary (and annoying) construct, so I make effort to code without it. If there is a null somewhere, someone made a mistake, so the best thing is to just barf out instead of pretending everything is ok.

In the end it depends on what you consider to be the semantics of the method. If the method is supposed to accept nulls, then you should pick option number two. If the method is supposed to only accept real arguments (which I prefer), then you should pick option number one.

Answer 6

If the method body handles the null obj properly (in other words, obj != null is not a requirement), then there's no need to throw an exception.

In all other cases: Throw. Let the client take responsibility for their flawed input.

Answer 7

It really depends on what your invariants are. If the parameter is optiona, then ignoring a null parameter is just fine, but if the parameter is required then that will hide a bug in your application. Also, and depending on the language, if the invariant is bad enough you may consider a third option: abort the application.

All discussions on whether to use or not exceptions can always be mapped to the decision on whether the situation is exceptional or not, and if it is exceptional, throwing or rather aborting the application depends on whether it is recoverable or not.

Answer 8

Always Throw, except in debugging/diagnostic code. It is most embarassing to have a NullPointerException that occurs in production code at a point where only a debugging message should be generated, e.g.

log.debug("id of object is " + obj.getId())

where the logger is turned off, and obj is null.

Answer 9

It is highly subjective, but I always prefer to just ignore non-fatal or recoverable errors. Put them in logs, if you must, but if you know how to continue - please do so.

Note, that when I say fatal, it actually depends on the function itself. Say, there's API function that gets ID and handful of other parameters. Suppose, that this ID also can be guessed from those other stuff that is passed in. API function should guess it if it can but the function somewhere inside that does all the work should get non-null ID and throw otherwise. Because for high level API function it is not fatal, it knows how to guess it, but for low level function it is fatal, it supposed to do something with that ID and with null value it can't continue.

All fatal errors should be noted, of course.

Answer 10

Throwing an exception (if null is an error) seems far better than silently ignoring an error.

There is a third option you can consider:

void PrintType(object obj)
{
    Console.WriteLine(obj.GetType().Name);
}

This also throws an exception when obj is null. The advantage of this, is that less code is involved. The disadvantage of this approach is that it is more difficult to tell whether obj can be null.

Answer 11

Id go for

void PrintType(object obj) { Console.WriteLine(obj.GetType().Name); }

Answer 12

Third option, half in pseudocode:

// To Throw A Clean Error
void PrintType(object obj)
{
    if(obj == null) 
    {
        throw new ArgumentNullException(STANDARD_ERROR_MESSAGE, obj)
    }

    Console.WriteLine(obj.GetType().Name);    
}

Either catch all errors and wrap them in a single place, so the user sees standard text:

There has been an error. If this error persists, please contact an administrator.

Or throw a select few errors, all of which are user-friendly, and display them directly to the user. "A connection error has occurred." "An authentication error has occurred." "A system error has occurred." And so on.

On the backend, have all errors and their stack trace logged, so you can use the debugging information that way.

Answer 13

It really depends on what the function is defined to do. The most important aspect is to have a clearly defined behavior and for the function to implement it correctly.

Now, if the question is whether is better to define the function to accept null and print it out, or to not accept it and throw an exception, I would say the latter, because it's probably less error prone for the user to check for null before calling the function, if that is a possibility.

Answer 14

If you api if exposed outside, do always argument checking and throw a argument based exception so the api user can get the result.