tags:

views:

774

answers:

15
+15  Q: 

Never use Nulls?

We are currently going through the long process of writing some coding standards for C#.

I've written a method recently with the signature

string GetUserSessionID(int UserID)

GetUserSession() returns null in the case that a session is not found for the user.

in my calling code... I say...

string sessionID = GetUserSessionID(1)
if (null == sessionID && userIsAllowedToGetSession)
{
    session = GetNewUserSession(1);
}

In a recent code review, the reviewer said "you should never return null from a method as it puts more work on the calling method to check for nulls."

Immediately I cried shenanigans, as if you return string.Empty you still have to perform some sort of check on the returned value.

if (string.Empty == sessionID)

However, thinking about this further I would never return null in the case of a Collection/Array/List. I would return an empty list.

The solution to this (I think) would be to refactor this in to 2 methods.

bool SessionExists(int userID);

and

string GetUserSessionID(int UserID);

This time, GetUserSessionID would throw a SessionNotFound exception (as it should not return null)

now the code would look like...

if(!SessionExists(1) && userIsAllowedToGetSession))
{
   session = GetNewUserSession(1);
}
else
{
   session = GetUserSessionID(1);
}

This now means that there are no nulls, but to me this seems a bit more complicated. This is also a very simple example and I was wondering how this would impact more complicated methods.

There is plenty of best-practice advise around about when to throw exceptions and how to handle them, but there seems to be less information regarding the use of null.

Does anyone else have any solid guidelines (or even better standards) regarding the use of nulls, and what does this mean for nullable types (should we be using them at all?)

Thanks in advance,

Chris.

=====

Thanks everyone! LOTS of very interesting discussion there.

I've given the answer to egaga as I like thier suggestion of Get vs Find as a coding guideline, but all were interesting answers.

+26  A: 

nulls are definitely better, i.e., more honest, than "magic values". But they should not be returned when an error has happened - that's what exceptions are made for. When it comes to returning collections... better an empty collection than null, I agree.

ammoQ
I think exceptions should be reserved for unusual errors rather than just any error e.g. Network connection has disappeared - throw an exception, no matches for that search, return a variable (bool, null or otherwise) that indicates this quite likely to happen error has happened.
morechilli
+1  A: 

NULL means unknown/undetermined.

You mostly need it when working with a database where data is simply 'not determined (yet)'. In your application, NULL is a good return type for non-exceptional but not-default behavior.

You use NULL when the output is not the expected but not serious enough for an exception.

Mark
+7  A: 

A possible practice is to use get prefix for methods that throw an exception if result is not found, and find prefix, if null is possible. Thus it's easy to see in client side whether the code could have a problem dealing with null.

Of course one should avoid nulls, and Andrej Heljsberg has said in an interview that if C# was created now, it would have better ways dealing with nullability. http://www.computerworld.com.au/article/261958/-z_programming_languages_c?pp=3&fp=&fpid=

egaga
+1 for the subtle semantic difference between find and get.
TheFogger
I would call it non-standard, rather than subtle...
Matthew Flaschen
Nice exactly what I was thinking of saying. When calling a function called FindSomething it definitely makes you think you're going to need to handle the case when the something isn't found.
Scott Langham
+4  A: 

Keep things Simple. Make it as painless as possible for the consumer of your type. It's a tradeoff : time to implement and benefits gained.

  • When dealing with things like fetching lists/collections, return an empty list as you rightly pointed out.
  • Similarly for cases where you need to return a 'null' value to signify 'not-found' - try using the Null Object pattern for a heavily used type. For rarely used types, I guess you could live with a few checks for null or -1 (String.IndexOf).

If you have more categories, please post as comments and I'll attempt solutions.

Gishu
+2  A: 

One solution might be to declare a simple return type for such things:

class Session
{
    public bool   Exists;
    public string ID;
}

Session GetUserSession(int userID) {...}

...

Session session = GetUserSessionID(1);

if (session.Exists)
{
    ... use session.ID
}

I do generally like to avoid nulls, but strings are a bit of a special case. I often end up calling string.IsNullOrEmpty(). So much so that we use an extension method for string:

static class StringExtensions
{
    public static bool IsNullOrEmpty(this string value)
    {
        return string.IsNullOrEmpty(value);
    }
}

Then you can do:

string s = ... something

if (s.IsNullOrEmpty())
{
 ...
}

Also, since we're on the subject of coding style:

What's with the unnatural-looking "if (null == ...)" style? That's completely unneccessary for C# - it looks like someone's brought some C++ baggage into the C# coding styles!

Matthew Watson
I really want to upvote this for the mention of IsNullOrEmpty; but I'm shocked that you went to such a length to get around a static class call and then commented about "baggage".
overslacked
The "baggage" I referred to was related to C++ idioms being imported into C# coding styles; it's not really related to using extension methods!I first saw the IsNullOrEmpty() extension mentioned by Jon Skeet (you may have heard of him perhaps?).Its pros and cons are discussed somewhat here:http://stackoverflow.com/questions/790810/is-extending-string-class-with-isnullorempty-cofusing
Matthew Watson
(Further comment) But yes, the general consensus is that you shouldn't extend methods so you can apparently call them on null methods. :)
Matthew Watson
I understand what you were referring to; but complaining that a "backwards" equality test looks unnatural when you seem to be explicitly testing an existing object instance for existence seems to me just a touch ironic.At any rate, I think yours and ammoQ's answers are the best (and we all have baggage, I have plenty of my own learned/preferred ways of doing things!).
overslacked
+1  A: 

The design by contract answer you have would also be my solution:

if (myClass.SessionExists)
{
   // Do something with myClass.Session
}
Chris S
+3  A: 
SharePoint Newbie
+9  A: 

returning null is fine and the following is concise and easy to understand:

var session = GetUserSessionID(1) ?? GetNewUserSession(1);
hopethisworks
+4  A: 

In my opinion you shouldn't rule out the use of null as a return value. I think it is valid in many cases. But you should carefully consider the pro's and con's for every method. It totally depends on the purpose of the method and the expectations callers might have.

I personally use a null return value a lot in cases where you might expect that the method does not return an abject (i.e. a database search by the primary key, might return exactly one instance or none / null). When you can correctly expect a method to return a value, you should use an exception.

In your particular example I think it depends on the context of the system. If the call for instance is only made from code where you might expect a logged in user, yo should throw an exception. If however it is likely that no user is logged in and you therefore don't have a session id to return, you should choose to return null.

Jaap Coomans
+3  A: 

I'm wary of returning nulls myself, though in your example it certainly seems like the right thing to do.

What is important is being clear about nullability both for arguments and return values. This must be specified as part of your API documentation if your language can't express this concept directly. (Java can't, I've read that C# can.) If an input parameter or return value can be null, be sure to tell users what that means in the context of your API.

Our largest code base is a Java app that has grown steadily over the past decade. Many of the internal APIs are very unclear about their behavior WRT null. This leads to cancerous growth of null-checks all over the place because everyone is programming defensively.

Particularly ugly: functions which return collections returning null instead of an empty collection. WTF!? Don't do that!

In the case of Strings be sure to distinguish between Strings that must be there, but can be empty and strings that are truly optional (can be null). In working with XML we come across this distinction often: elements that are mandatory, but can be empty, versus elements that are optional.

One part of the system, which provides querying over XML-like structures for implementing business rules, is very radical about being null-free. It uses the Null Object Pattern everywhere. In this context, this turns out to be useful because it allows you to do things like:

A.children().first().matches("Blah")

The idea being that this expression should return true if the first child of A is named "Blah". It won't crash if A has no children because in that case first() returns a NullNode which implements the Node interface, but never 'matches' anything.

In summary:

  • Be clear about the meaning and permissibility of null in your APIs.
  • Use your brain.
  • Don't be dogmatic.
bendin
+4  A: 

Why don't you use the Null design pattern instead?

Geo
+1  A: 

We always return empty lists / collections and do not return NULL lists / collections because handling empty collections/lists reduces the burden on the calling functions.

Nullable - nullables are particularly useful when you want to avoid handling the "null" values from the databases. You can simply use GetDefaultOrValue for any Nullable type without worrying about the database value being null in your business objects. We define Nullables typically ONLY in our business objects and that too where we really want to escape the db null values check, etc.

Vikram
+1  A: 

I prefer returning empty collections instead of nulls, because that helps avoid cluttering the caller code like the following:

if( list != null) {
    foreach( Item item in list ) {
         ...
    }
}
azheglov
A: 

The inventor of nulls thinks that they are a bad idea..

see: http://lambda-the-ultimate.org/node/3186

+1  A: 

I've had a similar issue, though I through an Exception and was told I should of returned null,

We ended up reading

this blog post about vexing exceptions

I'd recommend having a read.

danswain