126

Last week, we had a heated argument about handling nulls in our application's service layer. The question is in the .NET context, but it will be the same in Java and many other technologies.

The question was: should you always check for nulls and make your code work no matter what, or let an exception bubble up when a null is received unexpectedly?

On one side, checking for null where you are not expecting it (i.e. have no user interface to handle it) is, in my opinion, the same as writing a try block with empty catch. You are just hiding an error. The error might be that something has changed in the code and null is now an expected value, or there is some other error and the wrong ID is passed to the method.

On the other hand, checking for nulls may be a good habit in general. Moreover, if there is a check the application may go on working, with just a small part of the functionality not having any effect. Then the customer might report a small bug like "cannot delete comment" instead of much more severe bug like "cannot open page X".

What practice do you follow and what are your arguments for or against either approach?

Update:

I want to add some detail about our particular case. We were retrieving some objects from the database and did some processing on them (let's say, build a collection). The developer who wrote the code did not anticipate that the object could be null so he did not include any checks, and when the page was loaded there was an error and the whole page did not load.

Obviously, in this case there should have been a check. Then we got into an argument over whether every object that is processed should be checked, even if it is not expected to be missing, and whether the eventual processing should be aborted silently.

The hypothetical benefit would be that the page will continue working. Think of a search results on Stack Exchange in different groups (users, comments, questions). The method could check for null and abort the processing of users (which due to a bug is null) but return the "comments" and "questions" sections. The page would continue working except that the "users" section will be missing (which is a bug). Should we fail early and break the whole page or continue to work and wait for someone to notice that the "users" section is missing?

Community
  • 1
Stilgar
  • 1,504
  • 2
  • 10
  • 13
  • 64
    Fox Mulder principle: Trust no one. – yannis May 06 '12 at 15:59
  • 14
    @YannisRizos: that principle is a good one - it is so good that the creators of Java and C# let the language run time environment do this automatically. So normally there is no need to do explicit checks for *null* everywhere in your code in those languages. – Doc Brown May 06 '12 at 16:26
  • 4
    possible duplicate of [Should a method validate its parameters?](http://programmers.stackexchange.com/questions/64926/should-a-method-validate-its-parameters) – Martin York May 06 '12 at 19:06
  • I agree with tdammers answer. I believe checking for null is just wrong because you do not have a reasonable way to handle it. However in your scenario you can handle a section failing such as validating a user (or getting comments or whatever it may be) and having a "oh no an error has occurred" box. I personally log all the exceptions in my app and filter out certain exceptions like 404s and connection close unexpectedly –  May 06 '12 at 21:40
  • 2
    `assert(foo != null, "foo is web control within the repeater, there's no reason to expect it to be null, etc, etc...");` – zzzzBov May 06 '12 at 21:55
  • Any external data retrieval can fail. However, some of these failure scenarios are very difficult to test (i.e.: the DB is down, no company will let you take down a DB for you to test), and in those cases, it will most likely return a NULL to prevent the assumption of zero data returned. There is really only one case where I do not check for a null, and that is if I assigned the data in the code itself. If the data comes from somewhere, it can be NULL at some point. – Nelson Feb 03 '15 at 07:15
  • Use code contracts and add Ensures where appropriate (almost everywhere). – Andy Mar 13 '15 at 00:14
  • @DocBrown the intention of those creators was good, but the fact is `NullReferenceException` is the most notorious exception to debug and figure out *what* exactly was null. Sometimes it's easy to tell, sometimes very hard. – Ohad Schneider Jul 05 '17 at 11:59
  • 1
    throw exception when developing. hide exceptions when shipping. use assertions for this.. – M.kazem Akhgary May 15 '20 at 08:28

16 Answers16

114

The question is not so much whether you should check for null or let the runtime throw an exception; it is how you should respond to such an unexpected situation.

Your options, then, are:

  • Throw a generic exception (NullReferenceException) and let it bubble up; if you don't do the null check yourself, this is what happens automatically.
  • Throw a custom exception that describes the problem on a higher level; this can be achieved either by throwing in the null check, or by catching a NullReferenceException and throwing the more specific exception.
  • Recover by substituting a suitable default value.

There is no general rule which one is the best solution. Personally, I would say:

  • The generic exception is best if the error condition would be a sign of a serious bug in your code, that is, the value that is null should never be allowed to get there in the first place. Such an exception may then bubble all the way up into whatever error logging you have set up, so that someone gets notified and fixes the bug.
  • The default value solution is good if providing semi-useful output is more important than correctness, such as a web browser that accepts technically incorrect HTML and makes a best effort to render it sensibly.
  • Otherwise, I'd go with the specific exception and handle it somewhere suitable.
tdammers
  • 52,406
  • 14
  • 106
  • 154
  • What is the difference between specific exception and null in the grand scheme of things? The program still crashes if they are unhandled (i.e. there is no user interface for this case) – Stilgar May 07 '12 at 06:26
  • 2
    @Stilgar for the enduser there is no difference. for the developper a specific exception like "database-server is not reachable" is more intuitive that "null pointer excepteion" – k3b May 07 '12 at 07:16
  • Sure but the end result would still be that the application (or the specific page) will not work at all. – Stilgar May 07 '12 at 07:33
  • 35
    Failing hard and early means that there's less risk of things going more subtly wrong down the line, things like mis-authentication, corrupted data persisted to storage, information leakage, and other amusing things. – Lars Viklund May 07 '12 at 12:14
  • 1
    @Stilgar: There are two concerns here: unattended behavior, and maintainability. While for the unattended behavior there is little difference between a specific exception and a generic one, for maintainability it can make the difference between "oh, so that's why things blow up" and a multi-hour debugathlon. – tdammers May 07 '12 at 19:21
  • 4
    tdammers is exactly right. The "object reference not set to an instance of an object" is one of the most annoying exceptions I see with the most frequency. I would much rather see a specific exception whether that is an ArgumentNullException with the parameter name, or a custom exception "No Post record exists for user TK421." – Sean Chase Dec 08 '13 at 17:00
  • This is a great answer, but I do use `Debug.Assert(value != null, "Some useful info about what is wrong with the way this code is being used")` to make it clear that I am explicitly not checking for null-references. This prevents this situation from getting hidden in some logfile somewhere while you are writing new code that uses this code (well, I hope so at least!). – Wouter Simons Mar 05 '14 at 07:14
  • 3
    @k3b For the end-user as well, "database-server is not reachable" is very helpful. At least, for any end-user that has a bit of a clue about common problems - it may help him find that a cable's loose, the router needs restarting, etc. rather than getting angry about buggy software. – Julia Hayward Feb 03 '15 at 09:52
  • @SeanChase: Would there be any fundamental problem with using metadata to supply a bit more information about what part of an expression was being referenced when a null-reference trap was hit? What's annoying with the exception is that the compiler almost certainly has a lot more information than it lets on about. – supercat Mar 26 '16 at 21:24
  • When you say "The question is not so much whether you should check for null or let the runtime throw an exception" do you mean that to your view, every public method argument should be checked for nullity? – Ohad Schneider Jul 05 '17 at 12:11
62

IMHO trying to handle null values that you don't expect leads to overly complicated code. If you don't expect null, make it clear by throwing ArgumentNullException. I get really frustrated when people check if the value is null and then try to write some code that doesn't make any sense. The same applies to using SingleOrDefault (or even worse getting collection) when someone really expects Single and many other cases when people are afraid (I don't really know of what) to state their logic clearly.

empi
  • 811
  • 1
  • 5
  • 3
  • Instead of `ArgumentNullException`, one should use code contracts, (unless if it's a pre-2010 source code which doesn't support code contracts). – Arseni Mourzenko May 06 '12 at 18:40
  • I agree with you. If the null is not expected it should not be handled in strange ways but only reported. – Giorgio May 06 '12 at 18:56
  • 10
    if I read the question right, throwing an `ArgumentNullException` counts as "handling" the null value: it *prevents* a later null reference exception. – KutuluMike May 06 '12 at 21:19
  • @MichaelEdenfield: I don't think that counts as _real_ handling according to the given question (our goal is not to _make your code work no matter what_). To be honest, I often forget to write appropriate block that will throw if null is passed. However, I consider throwing NullArgumentException to be the best practice and the worst practice in my opinion is writing code that cannot handle null but instead of throwing exception returns e.g. another null as a method result (or empty string, or empty collection, or -1, or skips method execution if return type is void, etc.). – empi May 06 '12 at 21:53
  • @empi: I do not agree with your last observation. If a method uses null as a return value to indicate undefined, then it is perfectly OK to check it and return null: you just set a convention to represent undefined and write your code to propagate undefined when it is encountered. I think you are confusing the semantics (undefined and undefined propagation) with the implementation (using null, a special error code like -1, or exceptions). – Giorgio May 07 '12 at 08:08
  • 4
    @Giorgio, `ArgumentNullException` makes it very clear what the problem is and how to fix it. C# doesn't tend to use "undefined". If you call something in a way that's undefined, then the way you're calling it doesn't make sense. An exception is the proper way to indicate that. Now, sometimes I'll make parsing methods that return null if the `int` (for example) can't be parsed. But that's a case where an unparseable result is a legitimate possibility rather than a usage error. See also [this article](http://blogs.msdn.com/b/kcwalina/archive/2007/01/30/exceptionhierarchies.aspx). – Kyralessa May 07 '12 at 13:51
  • @Kyralessa: For that parsing example, the `TryParse` idiom is preferrable: return a `bool` indicating success or failure, and return the result as an `out` parameter. Returning `null` here is not a great option as it mixes your parsing return value and flow control return value. Also if you named your method `SomeType Parse(string)`, then the method isn't fulfilling its implicit contract given by the name of the method. – Merlyn Morgan-Graham May 21 '12 at 08:59
  • @MerlynMorgan-Graham, there's nothing wrong with doing it the way I describe if the method returns `Nullable`. The semantics of `Nullable` are not mysterious. – Kyralessa Jul 19 '12 at 14:56
  • @Kyralessa: Returning `Nullable` doesn't solve the null ambiguity problem (did the parse fail or succeed? Was the contents of the string empty? Was the input null?). The `TryParse` idiom solves this, letting you know whether you have a working object or not. The logic to determine if the parse succeeded or not belongs in the parse method, not the method that uses the result of the parse. You're going to have to check if the parse succeeded anyhow, you might as well have that be returned out-of-band from the output. – Merlyn Morgan-Graham Jul 20 '12 at 19:34
  • @MerlynMorgan-Graham, kindly go read [Nullable Structure](http://msdn.microsoft.com/en-us/library/b3h38hb0.aspx). "The Nullable structure supports using only a value type as a nullable type because reference types are nullable by design." If you get back a null, logically the parse must have been a failure, because the type you were parsing to can't be nullable. Whether it was a failure because you tried to parse "abc" or "" or something else doesn't matter for purposes of the Parse method I've described. A "Parse" method is not the place to enforce minimum length in an input field. – Kyralessa Jul 20 '12 at 20:11
  • @Kyralessa: I know how `Nullable` works, and it does not solve the problem the way you think it does. – Merlyn Morgan-Graham Jul 21 '12 at 01:58
  • @MerlynMorgan-Graham: The parameter sequence for the `TryXXX` idiom unfortunately doesn't work with covariant interfaces. Having e.g. `T TryGetSomething(..., out bool Success);` would allow for covariance and also allow the returned variable to be stored in a `var` declaration. – supercat Feb 03 '14 at 23:37
37

The habit of checking for null to my experience comes from former C or C++ developers, in those languages you have a good chance of hiding a severe error when not checking for NULL. As you know, in Java or C# things are different, using a null ref without checking for null will cause an exception, thus the error will not be secretly hidden as long as you don't ignore it on the calling side. So in most cases, checking explicitly for null does not make much sense and overcomplicates the code more than necessary. That's why I don't consider null checking to be a good habit in those languages (at least, not in general).

Of course, there are exceptions from that rule, here are those I can think of:

  • you want a better, human-readable error message, telling the user in what part of the program the wrong null reference occured. So your test for null throws just a different exception, with a different error text. Obviously, no error will get masked that way.

  • you want to make your program "fail more early". For example, you want to check for a null value of a constructor parameter, where the object reference otherwise would only be stored in a member variable and used later.

  • you can deal with the situation of a null ref safely, without the risk of subsequent faults, and without masking a severe error.

  • you want a completely different kind of error signaling (for example, returning an error code) in your current context

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 4
    "you want a better, human-readable error message" - that's the best reason to do it, in my opinion. "Object reference not set to an instance of an object" or "Null reference exception" is never as helpful as "Product not instantiated". – pdr May 06 '12 at 16:31
  • @pdr: Another reason to check is to make sure it is always detected. – Giorgio May 06 '12 at 18:57
  • 15
    "Former C developers", perhaps. "Former C++ developers" only if those themselves are recovering C developers. As a modern C++ developer, I would be very suspicious about code that uses naked pointers or reckless dynamic allocation. In fact, I'd be tempted to turn the argument around and say that C++ does *not* suffer the problem that the OP describes because its variables don't have the additional special null-ness property that Java and C# have. – Kerrek SB May 06 '12 at 21:47
  • 8
    Your first paragraph is only half of the story: yes, if you use a null reference in C#, you will get an exception, whereas in C++ you will get undefined behavior. But in well-written C#, you are stuck with _far_ more nullable references than in well-written C++. – James McNellis May 06 '12 at 22:33
  • The better the encapsulation, the better this answer applies. – radarbob Oct 11 '15 at 20:21
  • @KerrekSB totally right. The current GSL effort by Sutter and Stroustrup and others are also aiming for eliminating null pointers. Actually limiting the use of pointers to be correct. They are valid! But man times references are the way to go. In C# for instance you can't do that easily. – HankTheTank Mar 10 '20 at 19:02
15

Use asserts to test and indicate pre/postconditions and invariants for your code.

It makes it so much easier to understand what the code expects and what it can be expected to handle.

An assert, IMO, is a suitable check because it is:

  • efficient (usually not used in release) ,
  • brief (usually a single line) and
  • clear (precondition violated, this is a programming error).

I think self-documenting code is important, therefore having a check is good. Defensive, fail-fast, programming makes maintenance way easier, which is where usually most time is spent.

Update

Wrt your elaboration. It's usually good to give the end user an application that works well under bugs, i.e. shows as much as possible. Robustness is good, cause heaven only knows what will break at a critical moment.

However , developers & testers must be aware of bugs ASAP, so you probably want a logging framework with a hook that can display an alert to the user that there was a problem. The alert should probably be shown to all users, but can probably be tailored differently depending on the runtime environment.

Macke
  • 2,576
  • 16
  • 16
  • asserts not being present in release is a big negative for me, release is the exact time you want the added information – jk. Feb 03 '15 at 10:48
  • 1
    Well, feel free use an assert^h^h^hconditional throw function that is active in release too if you need that. I've rolled my own quite often. – Macke Feb 03 '15 at 19:30
8

Fail early, fail often. null is one of the best unexpected values you can get, because you can fail quickly as soon as you try to use it. Other unexpected values are not so easy to detect. Since null automatically fails for you whenever you try to use it, I would say that it does not require an explicit check.

DeadMG
  • 36,794
  • 8
  • 70
  • 139
  • 31
    I hope you meant: Fail early, fail fast, not often... – empi May 06 '12 at 16:56
  • 12
    The problem is that without explicit checks, a null value can sometimes propagate quite far before it causes an exception, and then it can be very hard to find out where it originated. – Michael Borgwardt May 06 '12 at 16:57
  • @MichaelBorgwardt Isn't that what stack traces are for? – Roman May 08 '12 at 20:28
  • 7
    @R0MANARMY: stack traces don't help when a null value gets saved to some field and the NullPointerException is thrown much later. – Michael Borgwardt May 08 '12 at 20:30
  • @R0MANARMY even if you were able to trust the line number in the stack trace (in many cases you can't) some lines contain multiple variables that might be null. – Ohad Schneider Jul 05 '17 at 12:22
7

  • Checking for a null should be your second nature

  • Your API will be used by other people and their expectation is likely to be different from yours

  • Thorough unit tests normally highlight a need for a null reference check

  • Checking for nulls doesn't imply conditional statements. If you worry that null checking makes your code less readable, then you may want to consider .NET code contracts.

  • Consider installing ReSharper (or similar) to search your code for missing null reference checks

CodeART
  • 3,992
  • 1
  • 20
  • 23
  • 1
    Using code contracts for preconditions is simply glorified conditional statements. – user May 07 '12 at 11:28
  • Yes, I agree, but I also think that code looks cleaner when using code contracts, i.e. its readability is improved. – CodeART May 07 '12 at 11:30
7

My general approach is to never test for an error condition that you do not know how to handle. Then the question that needs to be answered in each specific instance becomes: can you do something reasonable in the presence of the unexpected null value?

If you can continue safely by substituting another value (an empty collection, say, or a default value or instance), then by all means do so. @Shahbaz's example from World of Warcraft of substituting one image for another falls into that category; the image itself is likely just decoration, and has no functional impact. (In a debug build, I'd use a screaming color to draw attention to the fact that substitution has occured, but that depends greatly upon the nature of the application.) Log details about the error, though, so you know it's occuring; otherwise, you'll be hiding an error that you probably want to know about. Hiding it from the user is one thing (again, assuming processing can continue safely); hiding it from the developer is quite another.

If you cannot continue safely in the presence of a null value, then fail with a sensible error; in general, I prefer to fail as soon as possible when it's obvious that the operation cannot succeed, which implies checking preconditions. There are times when you don't want to fail immediately, but defer failure for as long as possible; this consideration comes up for example in cryptography-related applications, where side-channel attacks might otherwise divulge information you do not want known. At the end, let the error bubble up to some general error handler, which in turn logs relevant details and displays a nice (however nice they can be) error message to the user.

It's also worth noting that the proper response may very well differ between testing/QA/debug builds, and production/release builds. In debug builds, I would go for failing early and hard with very detailed error messages, to make it completely obvious that there is a problem and allow a post-mortem to determine what needs to be done to fix it. Production builds, when faced with safely recoverable errors, should probably favor recovery.

Community
  • 1
user
  • 2,703
  • 18
  • 25
  • Of course there is general error handler up the chain. The problem with logs is that there may be no one to check them for very long time. – Stilgar May 07 '12 at 16:45
  • @Stilgar, if there is no one to check the logs, then that is not a problem with the existence or absence of null checks. – user May 08 '12 at 07:35
  • 2
    There is a problem. The end users may no notice that the system does not work correctly for a long time if there are null checks that simply hide the error and write it to the logs. – Stilgar May 08 '12 at 07:42
  • @Stilgar, that depends entirely on the nature of the error. As I said in the post, only **if you can continue safely** should the unexpected null be replaced/ignored. Using one image instead of another in a release build (in debug builds, fail as early and as hard as possible so the problem becomes obvious) is a very different beast than returning invalid results for a calculation, for example. (Edited answer to include that.) – user May 08 '12 at 07:50
  • 1
    I'd go with failing fast and early even in production unless you can somehow log and report the error (without relying too much on savvy users). Hiding your deployed bugs from users is one thing - hiding them from yourself is dangerous. Also allowing the user to live with subtle bugs can erode their confidence in your application. Sometimes it is better to bite the bullet and let them know there was a bug *and* that you fixed it quickly. – Merlyn Morgan-Graham May 21 '12 at 09:24
4

I would consider the question from the point of view of semantics, i.e. ask myself what a NULL pointer or a null reference argument represents.

If a function or method foo() has an argument x of type T, x can be either mandatory or optional.

In C++ you can pass a mandatory argument as

  • A value, e.g. void foo(T x)
  • A reference, e.g. void foo(T& x).

In both cases you do not have the problem of checking a NULL pointer. So, in many cases, you do not need a null pointer check at all for mandatory arguments.

If you are passing an optional argument, you can use a pointer and use the NULL value to indicate that no value was given:

void foo(T* x)

An alternative is to use a smart pointer like

void foo(shared_ptr<T> x)

In this case, you probably want to check the pointer in the code, because it makes a difference for method foo() whether x contains a value or no value at all.

The third and last case is that you use a pointer for a mandatory argument. In this case, calling foo(x) with x == NULL is an error and you have to decide how to handle this (the same as with an out-of-bounds index or any invalid input):

  1. No check: if there is a bug let it crash and hope this bug appears early enough (during testing). If it doesn't (if you fail to fail early enough), hope that it won't appear in a production system because the user experience will be that they see the application crash.
  2. Check all arguments at the beginning of the method and report invalid NULL arguments, e.g. throw an exception from foo() and let some other method handle it. Probably the best thing to do is to show the user a dialog window saying that the application has encountered an internal error and is going to be closed.

Apart from a nicer way of failing (giving the user more information), an advantage of the second approach is that it is more likely that the bug will be found: the program will fail every time the method is called with the wrong arguments whereas with approach 1 the bug will only appear if a statement dereferencing the pointer is executed (e.g. if the right branch of an if statement is entered). So approach 2 offers a stronger form of "fail early".

In Java you have fewer choices because all objects are passed using references which can always be null. Again, if the null represents a NONE value of an optional argument, then checking the null will (probably) be part of the implementation logic.

If the null is an invalid input, then you have an error and I would apply similar considerations as for the case of C++ pointers. Since in Java you can catch null pointer exceptions, approach 1 above is equivalent to approach 2 if method foo() dereferences all input arguments in all execution paths (which probably occurs very often for small methods).

Summarizing

  • Both in C++ and Java, checking if optional arguments are null is part of the program's logic.
  • In C++, I would always check mandatory arguments to ensure that a proper exception is thrown so that the program can handle it in a robust way.
  • In Java (or C#), I would check mandatory arguments if there are execution paths that won't throw in order to have a stronger form of "fail early".

EDIT

Thanks to Stilgar for more details.

In your case it seems that you have null as the result of a method. Again, I think you should first clarify (and fix) the semantics of your methods before you can take a decision.

So, you have method m1() calling method m2() and m2() returns a reference (in Java) or a pointer (in C++) to some object.

What is the semantics of m2()? Should m2() always return a non-null result? If this is the case, then a null result is an internal error (in m2()). If you check the return value in m1() you can have a more robust error handling. If you don't, you will probably have an exception, sooner or later. Maybe not. Hope that the exception is thrown during testing and not after deployment. Question: if m2() should never return null, why is it returning null? Maybe it should throw an exception instead? m2() is probably buggy.

The alternative is that returning null is part of the semantics of method m2(), i.e. it has an optional result, which should be documented in the the Javadoc documentation of the method. In this case it is OK to have a null value. Method m1() should check it as any other value: there is no error here but probably checking if the result is null is just part of the program logic.

Giorgio
  • 19,486
  • 16
  • 84
  • 135
  • In out case it was not the argument that was null. We made a database call for something that was expected to be present but in practice it was not present. The question is if we should check that every object exists or we should check only if we expect it to be missing. – Stilgar May 06 '12 at 18:05
  • So the result returned by a method was a reference to an object and null was the value used to represent the result: NOT FOUND. Do I understand correctly? – Giorgio May 06 '12 at 18:07
  • I have updated the question with additional details. – Stilgar May 06 '12 at 18:26
3

Sure NULL is not expected, but there are always bugs!

I believe you should check for NULL whether or not you don't expect it. Let me give you examples (although they are not in Java).

  • In World of Warcraft, due to a bug the image of one of the developers appeared on a cube for many objects. Imagine if the graphics engine didn't expect NULL pointers, there would have been a crash, while it was turned into a not so fatal (even funny) experience for the user. The very least is, you wouldn't have unexpectedly lost your connection or any of your save points.

    This is an example where checking for NULL prevented a crash and the case was silently handled.

  • Imagine a bank teller program. The interface does some checks and sends input data to an underlying part to perform the money transfers. If the core part expects not to receive NULL, then a bug in the interface can cause a problem in the transaction, possibly some money in the middle getting lost (or worse, duplicated).

    By "a problem" I mean a crash (as in crash crash (say the program is written in C++), not a null pointer exception). In this case, the transaction needs to roll back if NULL is encountered (which of course would not be possible if you didn't check the variables against NULL!)

I'm sure you get the idea.

Checking validity of arguments to your functions doesn't clutter up your code, as that is a couple checks at the top of your function (not spread in the middle) and greatly increases robustness.

If you have to choose between "the program tells the user that it has failed and gracefully shuts down or roll back to a consistent state possibly creating an error log" and "Access Violation", wouldn't you choose the first one? That means every function should be prepared for being called by a buggy code rather than expecting everything is correct, simply because bugs always exist.

Shahbaz
  • 896
  • 1
  • 10
  • 20
  • Your second example disproves your point. In a bank transaction if anything goes wrong the transaction should abort. It is precisely when you cover the error that money can be lost or duplicated. Checking for null would be like "The customer sends null money... well I'll just send $10 (the equivalent of the image of the developer)" – Stilgar May 07 '12 at 06:23
  • @Stilgar, The contrary! Checking for NULL would be abort with message. Not checking for NULL would be **crash**. Now crash in the beginning of transaction may not be bad, but in the middle can be catastrophic. (I didn't say the bank transfer should handle the error like the game! Just the fact that it _should_ handle it) – Shahbaz May 07 '12 at 08:38
  • 2
    The whole point of a "transaction" is that it can't be half completed :) If there is an error then it is rolled back. – Stilgar May 07 '12 at 16:33
  • This is terrible advice for anything where transactional semantics are actually required. You should make all your transactions be atomic and idempotent, otherwise any error you have will flat out guarantee lost or duplicated resources (money). If you have to deal with non-atomic or non-idempotent operations, you should wrap it very carefully in layers that guarantee atomic and idempotent behavior (even if you have to write those layers yourself). Always think: what happens if a meteor hits the web server at the moment this transaction is going on? – Merlyn Morgan-Graham May 21 '12 at 09:11
  • @MerlynMorgan-Graham, that's exactly what I'm saying. I don't understand how my text is confusing, but what I'm saying is that "you **should** check for NULL even if you don't expect it because a meteor might have hit the web server". That way, you can rollback or whatever you want to do. If you don't check for NULL, you crash. How is that bad advice?! Or like stilgar himself says above, "if there is an error...", well how can you check for error, if you don't check for NULL? – Shahbaz May 21 '12 at 11:43
  • @Shahbaz: "Now crash in the beginning of transaction may not be bad, but in the middle can be catastrophic" - if this is true that this would be catastrophic, then you didn't design your transactional semantics correctly. A "crash" (exception being thrown) in the middle is exactly what you want to have happen, so the transaction will be rolled back and none of it will be committed. Checking for null and throwing your own exception should be identical to a null reference exception in this respect. It is a silent non-abort you need to avoid, so you *should* check for null, then *throw*. – Merlyn Morgan-Graham May 21 '12 at 19:37
  • @Shahbaz: The point is that "throw an exception" and "crash" should have identical behavior with respect to that transaction, and with respect to a re-submitted version of the transaction. If they don't identical behavior, then your transaction isn't atomic/idempotent enough. – Merlyn Morgan-Graham May 21 '12 at 19:40
  • @MerlynMorgan-Graham, for the 100th time, I am also saying you **should** check for NULL. There is no disagreement here. (And by the way, not every language throws an exception when accessing NULL, e.g. C++) – Shahbaz May 21 '12 at 21:27
  • @Shahbaz: The clarification that you're talking about cases that give a silent failure when a null value is passed makes more sense. But your first Warcraft example seems to advocate that functionality (where no exception is thrown), and your second example seems to imply that throwing an exception would *cause* the problem. Your clarification in comments makes more sense. Maybe it would help if you edit the answer to clear up the confusion. – Merlyn Morgan-Graham May 21 '12 at 23:26
  • 1
    @MerlynMorgan-Graham, I did so. Hope that clears up the confusion. – Shahbaz May 22 '12 at 09:33
2

As the other answers pointed If you don't expect NULL, make it clear by throwing ArgumentNullException. In my view, when you are debugging the project it helps you to discover the faults in the logic of your program sooner.

So you are going to release your software, if you restore those NullRefrences checks, you don't miss anything on the logic of your software, it just makes double sure that a severe bug won't pop up.

Another point is that if NULL checking is on the parameters of a function, then you can decide if the function is the right place to do it or not; if not, find all references to the function and then before passing a parameter to the function review probable scenarios which may lead to a null reference.

Ahmad
  • 1,806
  • 3
  • 20
  • 32
1

Every null in your system is a ticking time bomb waiting to be discovered. Check for null at the boundaries where your code interacts with code you don't control, and forbid null within your own code. This rids you of the problem without naively trusting foreign code. Use exceptions or some sort of Maybe/Nothing type instead.

Doval
  • 15,347
  • 3
  • 43
  • 58
0

While a null pointer exception will eventually be thrown it will often be thrown far from the point where you got the null pointer. Do yourself a favor and shorten the time it takes to fix the bug by at least logging the fact that you git a null pointer as soon as possible.

Even if you don't know what to do now, logging the event will save you time later. And maybe the guy who gets the ticket will be able to use the time saved to figure out the proper response then.

Jon Strayer
  • 666
  • 5
  • 7
-1

Since Fail early, fail fast as stated by @DeadMG (@empi) is not pssible because the web-application must work well under bugs you should enforce the rule

no exception catching without logging

so you as developpers are aware of potential problems by inspecting the logs.

if you are using a logging framework like log4net or log4j you can setup an additional special logging output (aka appender) that mail you errors and fatalerrors. so you stay informed.

[update]

if the enduser of the page should not be aware of the error you can setup an appender that mail you errors and fatalerrors you stay informed.

if it is ok that the enduser of the page sees cryptic errormessages you can setup an appender that puts the errorlog for the pageprocessing at the end of the page.

No matter how you use the logging if you swallow the exception the programm continue with data that makes sense and the swallowing is no more silent.

k3b
  • 7,488
  • 1
  • 18
  • 31
  • 1
    The question is what happens to the page? – Stilgar May 07 '12 at 07:45
  • How do we know that the data makes sense and the system is in a consistent state. After all the error was unexpected so we don't know what the impact was. – Stilgar May 07 '12 at 16:37
  • "Since Fail early, fail fast as stated by @DeadMG (@empi) is not pssible because the web-application must work well under bugs you should enforce the rule": One could always catch all exceptions (catch (Throwable t)) and redirect to some error page. Wouldn't this be possible? – Giorgio May 19 '12 at 05:58
-1

There are already good answers to this question, may be one addition to them: I do prefer assertions in my code. If your code can work only with valid objects, but nut with null, you should assert on null value.

Assertions in this kind of situation would let any unit and/or integration tests fail with the exact message, so you can nail down the problem pretty fast prior to production. If such an error slips through the tests and goes to production (where assertions should be deactivated) you gonna receive NpEx, and a severe bug, but it is better, than some minor bug which is much more fuzzy and popping up somewhere else in the code, and would be more expensive to fix.

Further more if someone must work with your code assertions tells him about your assumptions you made during designing/writing your code (in this case: Hey dude, this object must be presented!), which makes it easier to maintain.

GeT
  • 193
  • 5
  • Could you please write something to down-vote? I would appreciate a discussion. Thanx – GeT May 11 '12 at 20:39
  • I didn't down vote, and I agree with your general idea. The language needs some work though. Your assertions define the contract of your API, and you fail early/fail fast if those contracts are violated. Doing this at the skin of your application will let users of your code know they did something wrong in their code. If they see a stack trace down in the recesses of your code, they will likely think your code is buggy - and you might think so too until you get a solid repro and debug it. – Merlyn Morgan-Graham May 21 '12 at 09:18
-1

I normally check for nulls but I "handle" unexpected nulls by throwing an exception that gives a bit more detail about exactly where the null occurred.

Edit: If you get a null when you don't expect one something is wrong--the program should die.

Loren Pechtel
  • 3,371
  • 24
  • 19
-1

It depends on the language implementation of exceptions. Exceptions allow you to take some action to prevent damage to data or to cleanly release resources in the event your system enters an unanticipated state or condition. This is different from error handling which are condition you can anticipate. My philosophy is that you should have as little exception handling as possible. It's noise. Can your method do anything reasonable by handling the exception? Can it recover? Can it repair or prevent further damage? If you are just going to catch the exception and then return from the method or fail in some other harmless way then there was really no point in catching the exception. You would have only added noise and complexity to your method, and you may be hiding the source of a fault in your design. In this case I would let the fault bubble up and be caught by an outer loop. If you can do something such as repair an object or mark it as corrupt or release some critical resource such as a lock file or by cleanly closing a socket then this is a good use of exceptions. If you actually expect the NULL to show up frequently as a valid edge case then you should handle this in the normal logic flow with if-the-else or switch-case statements or whatever, not with an exception handling. For example, a field disabled in a form may be set to NULL, which is different than an empty string representing a field left blank. This is an area where you must use good judgment and common sense. I have never heard of good solid rules of thumb for how to deal with this problem in every situation.

Java fails in its exception handling implementation with "checked exceptions" where every possible exception that might be raised by objects used in a method must be caught or declared in the "throws" clause of the method. The is the opposite of C++ and Python where you may choose to handle exceptions as you see fit. In Java if you modify the body of a method to include a call that may raise an exception you are faced with the choice of adding explicit handling for an exception that you may not care about thus adding noise and complexity to your code, or you must add that exception to the "throws" clause of the method you are modifying which not only adds noise and clutter but changes the signature of your method. You must then go an modify code wherever your method is used to either handle the new exception your method now may raise or add the exception to the "throws" clause of those methods thus triggering a domino effect of code changes. The "Catch or Specify Requirement" is must be one of the most annoying aspects of Java. The exception exception for RuntimeExceptions and the official Java rationalization for this design mistake is lame.

That last paragraph had little to do with answering your question. I just hate Java.

-- Noah

Noah Spurrier
  • 107
  • 1
  • this post is rather hard to read (wall of text). Would you mind [edit]ing it into a better shape? – gnat May 23 '15 at 19:40