13

Various programming books suggest that methods should not return null values (Clean Code for example). Instead of returning null default values (0 or empty string or empty object) should be returned or an exception should be thrown. This is recommended in order to avoid many != null checks or to avoid NullPointerException.

I really don't understand how this helps. If you return a default value you have to make != DEFAULT checks; more over, the default value might mean something (like 0 for a credit amount). Throwing a specific exception is not worth with, because if you don't handle your code well, an exception is thrown anyway - NullPointerException.

I've thought about this when I found a bug caused by a method which returned an empty object (e.g. new MyBean()) instead of returning null. The code didn't worked functionally and the logs didn't warn anything, so detection and fixing was not that easy; with null a NullPointerException would have been thrown so spotting and fixing the bug would have been trivial.

The only case where it makes sense to me to return a default value instead of returning null is when returning arrays/collections; here they are usually consumed for each element in collection so if its empty nothing happens but if it's null NPE is thrown.

Random42
  • 10,370
  • 10
  • 48
  • 65

4 Answers4

13

Various programming books suggest that methods should not return null values (Clean Code for example). Instead of returning null default values (0 or empty string or empty object) should be returned or an exception should be thrown. This is recommended in order to avoid many != null checks or to avoid NullPointerException.

It is worth mentioning and a common misunderstanding, that Mr. Martin did not claim that you should always throw an exception and should never return an error value. In fact, the relevant passage reads quite different if you read it carefully (I don't have the english version here, so I have to translate it back, sorry):

If you are about to return a null pointer, you should consider throwing an exception instead.

Note that it says "consider", not "thou shalt".

Another good read are these two:


PS: Original text from my german edition:

Wenn Sie versucht sind, einen Nullzeiger zu liefern, sollten Sie stattdessen erwägen, eine Exception zu werfen.

JensG
  • 2,423
  • 1
  • 19
  • 25
  • I did not quote saying that an exception should be always thrown: "instead of returning null, default values or and exception [...]". – Random42 Feb 09 '14 at 12:40
  • +1: The biggest point made here I think is the point that `Clean Code` is not a gospel. The author knows this and is providing tips rather than commandments. – Joel Etherton Feb 09 '14 at 14:01
  • @m3th0dman: I am aware of that. – JensG Feb 09 '14 at 14:10
7

In my opinion, for single value returning methods, null is far superior to returning an empty or default instance of a type.

Not only can default values mask errors but, depending on the complexity of the types in question, they can create complex, hard to document, and tightly coupled dependencies between functions. API producers and API consumers must maintain a consistent notion of which values represent erroneous results.

On the other hand, when it comes to collections, returning an empty collection is far superior to returning null, because the expectation of the caller of the method is that a variable number of results may be yielded, so an empty collection represents a conceptually valid result and is symmetric with the concept of the empty set in mathematics.

All that said, however, exceptions have their place, especially when a method returns void or unit or a collection or iterator.

Aluan Haddad
  • 678
  • 4
  • 9
  • 1
    This is why to be clear for everyone you should always throw an exception when you cannot return the value required by your return type. Throwing exception is clear and come with possibility to send a message to the developer. returning null or default will always be subject to debate. Throwing exception will close all debate. Point. Or be clear on the name of you method like Entity Framework is and create a version of ```myMethodOrDefault()``` – Bastien Vandamme Jun 28 '19 at 04:36
3

It is critically important to distinguish between methods that are expected to encounter error conditions, and those that are not. It is also just as important to have a clear understanding as to the contract for that method.

If the method is one that routinely encounters failure (such as say retrieving data from a database), then it needs a way to return success/failure as well as a way to return the data. My preference usually is to return true/false for the operation, use a separate call to retrieve the data and/or the status.

If the method is one that is not expected to encounter failure (such as returning an item from a collection by index), it should never return null. It should return a valid item, or raise an exception, or assert. Callers should never check the return result, because it will always be valid.

It may be convenient to return a default value rather than assert, but this should be reflected in the method contract, and there could be two methods (GetXxx and GetXxxOrDefault).

Widespread checking of return values against null is often symptomatic of deeper problems in the code. Advice found in more recent programming books is sound.


In response to comments, consider the case where a method is required to perform a lookup, computation, parse etc and failure is a routine occurrence. I dislike exceptions and the TryParse() form, because they break the flow of the code, while the FindOrDefault() form is a pain if default is a legal return value. I tend to prefer this wrapper:

public static U SafeLookup<T, U>(this Dictionary<T, U> dict, T key, U missing = default(U)) {
  U ret;
  if (dict.TryGetValue(key, out ret)) return ret;
  return missing;
}

A value is always returned, and the value to return on error can optionally be specified.

david.pfx
  • 8,105
  • 2
  • 21
  • 44
  • And a default value should only be returned if the caller can use that default value without any negative consequences. If a caller has to check anyway, then a clear error returned is much superior to having to check whether a value is the default value. – gnasher729 Jun 26 '16 at 09:31
  • 1
    @gnasher729: the point is that if you return a value, the caller should never have to check whether it's valid. If the caller should check, then the return should be the success/failure mentioned. As an aside, many functional languages return a 'Maybe' type or similar, which is a good approach. – david.pfx Jun 26 '16 at 11:43
  • "If the method is one that routinely encounters failure (such as say retrieving data from a database), then it needs a way to return success/failure as well as a way to return the data" this is not the best example. For me if you need to return success/failure as well as a way to return the data then you must consider your method will always return the data but be ready for an exception. Returning true false is not enough and you will just hide the real exception. Or you do like parsing method with a TryParse yuo can create a TryGetMyData version. Then why not TryGetDataOrDefault() :-) – Bastien Vandamme Jun 28 '19 at 04:42
  • This is actually a good point. If you think you must return the success at same time of your request/method then you must create a TryXxx version of your method. Default behavior is you must consider your method is doing what you ask and throe in case of error. Really Parse and TryParse is a good example. – Bastien Vandamme Jun 28 '19 at 04:47
  • 1
    I find most of this very sound reasoning but I strongly disagree with _"return true/false for the operation, use a separate call to retrieve the data and/or the status."_because, by splitting _checking_ from _retrieval_ you lose any guarantee that the check result still holds when you retrieve. This is especially problematic in multi-threaded environments but is problematic even in single-threaded code. – Aluan Haddad Sep 09 '20 at 20:02
  • @AluanHaddad: Then perhaps you misunderstand. The true return means a value is now available, false means it is not (but perhaps there is an error code for why). The new state is stable and perfectly thread-safe. This is command query separation. There is still room for throwing an exception if something bad happens. – david.pfx Sep 11 '20 at 00:39
2

I like the idea of throwing an exception instead of returning null or other error-value. Let's say I'm designing a new class that parses an XML of a certain format. I would like my class to have just one entry point, that is, a constructor that specifies the path to a valid XML file. In return, I get an object with a bunch of properties and methods that holds the content of my XML file and gives me some information about it.

Now, for example, how can I make sure that my path to the XML file was valid? I can have, say, a Load(string xmlPath) function in my class. What should be its return type? Should it return a bool or int or what? And, most interesting, how can I check that? Ideally I should make the Load function private, and don't even care that it exists. But if I want to check the result of Load, then the function should be made public. Or, pass a bool loadSuccesful variable to my constructor and fill that with the result of loading the file. But wait, what if I want to check my XML file against an XSD? I'll have to have another function, parseXmlSchema(string xmlPath) or something like that. What should it return on failure? And how can you get the information out of your class while keeping a clean interface at the same time? Other validations could also be necessary.

While, if you make these kinds of methods private, and throw an exception when something goes wrong (there really is no point in parsing your XML file if it's not valid), you can have a clean interface an use your class in a try/catch statement. You can have your class constructor with only one string parameter (or two, if you count the XSD schema). If all is well in the try statement, if nothing went wrong, no exceptions thrown, paths exists, etc. your program is happy. If you're currently in the middle layer of your app and get an FileNotFoundException from your lower layers, you can rethrow that and pass it to your user (GUI) to do something about it. The GUI layer could catch it at this moment and alert the user, through a messageBox, for example, and log it as a bonus. The user then tries another XML file, and on goes the story. If you catch all possible exceptions and display meaningfull information to the user or take the appropriate measures in the background, there is no need for your application to freeze (you've likely seen that), if that's what you're afraid of.

As a bonus, you can pass all kind of meaningful information at the point your exception was thrown, like messages, parameter names, line numbers, function name, etc. If you document your code (as you should), document the exceptions that a class/method can throw.

mihai
  • 307
  • 3
  • 11