CODEWITHSUNDEEP
c#null
ChrisV
ChrisV

Reputation: 2233

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.

Upvotes: 19

Views: 1421

Answers (15)

danswain
danswain

Reputation: 4177

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.

Upvotes: 1

user81302
user81302

Reputation: 78

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

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

Upvotes: 1

azheglov
azheglov

Reputation: 5523

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 ) {
         ...
    }
}

Upvotes: 2

Vikram
Vikram

Reputation: 6877

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.

Upvotes: 1

Geo
Geo

Reputation: 96987

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

Upvotes: 5

bendin
bendin

Reputation: 9574

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.

Upvotes: 3

Jaap Coomans
Jaap Coomans

Reputation: 1465

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.

Upvotes: 5

hopethisworks
hopethisworks

Reputation: 207

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

var session = GetUserSessionID(1) ?? GetNewUserSession(1);

Upvotes: 11

SharePoint Newbie
SharePoint Newbie

Reputation: 6082

Nulls are far better than magic values and make much more sense. You could also try and write a TryGetUserSession method.

bool TryGetUserSession(int sessionId, out session)

Also try not to write nulls == ???, as some developers find it harder to read.

Kind regards,

Upvotes: 3

Chris S
Chris S

Reputation: 65476

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

if (myClass.SessionExists)
{
   // Do something with myClass.Session
}

Upvotes: 1

Matthew Watson
Matthew Watson

Reputation: 109852

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!

Upvotes: 2

Gishu
Gishu

Reputation: 136683

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.

Upvotes: 4

egaga
egaga

Reputation: 21732

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=

Upvotes: 8

Mark van Straten
Mark van Straten

Reputation: 9425

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.

Upvotes: 0

Erich Kitzmueller
Erich Kitzmueller

Reputation: 36987

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.

Upvotes: 29

Related Questions