I seem to have fallen into some massive, massive trouble with NullReferenceExceptions

6
votes

Recently I'm developing a software that parses and displays XML information from a website. Simple enough right?

I'm getting LOADS of NullReferenceExceptions. For example, this method:

private void SetUserFriends(List<Friend> list)
{
    int x = 40;
    int y = 3;

    if (list != null)
    {
        foreach (Friend friend in list)
        {
            FriendControl control = new FriendControl();
            control.ID = friend.ID;
            control.URL = friend.URL;
            control.SetID(friend.ID);
            control.SetName(friend.Name);
            control.SetImage(friend.Photo);

            control.Location = new Point(x, y);
            panel2.Controls.Add(control);

            y = y + control.Height + 4;
        } 
    }
}

I had to wrap an ugly as sin If around the actual foreach loop in order to prevent an exception.

I feel I'm just putting bandaids on a flat tire instead of actually fixing the problem. Is there any way I can tackle this problem? Maybe a book I should read regarding programming patterns or what not?

Really, I'm lost. I'm probably asking the wrong questions.

6
c#.netnullreferenceexception
You should be looking at the code that's calling SetUserFriends. If you assume the list of friends should not be null (which is a fair enough assumption, I would say), then the bug is in whatever is passing in null. Use the debugger to look up the call stack when you get the exception. – Dean Harding
Better check why you have a null List reference instead of an empty List object. – BenV
This is a side note, but I'd argue for accepting IEnumerable<Friend>, so that the method does not require the caller to use a particular collection class. – Steven Sudit
@Steven or at the least an IList if you need list capabilities – Darko Z
@Darko: Yes, definitely. Whatever the minimally sufficient interface is. – Steven Sudit

6 Answers

15
votes

It sounds as if you're unsure what to do if you receive bad parameters in your methods. There's nothing inherently wrong with what you're doing now, but the more common pattern is to check parameters in the head of your method, throwing an exception if they're not what you're expecting:

if (list == null)
{
    throw new ArgumentNullException(list);
}

This is a common defensive programming pattern - check to make sure that the data you're provided passes basic sanity checks.

Now, if you're calling this method explicitly yourself, and you're finding this method receiving a null list parameter when you're not expecting it, it's time to look at the logic of the calling method. Myself, I prefer to pass an empty list when I have no elements, as opposed to null, to avoid special cases such as this.

4
votes

I'm probably going to get downvoted by the "no multiple exit" crowd but I usually handle that with a simple check right at the beginning of the method:

if (list == null || list.Count == 0) return;

this specifies the exit conditions and then you don't need to worry about multiple levels of indentation in your method. This only works if you can afford to swallow the fact that your list is null or empty - which can happen in some cases.

But I agree with codeka, in that you need to look at the calling code and work out if you can improve it from there.

2
votes

It seems like defensive programming and parameter validation is what you're looking for.

As others have said, simple parameter validation would work for you:

if (list == null)
    throw new ArgumentNullException("list");

Alternatively, if you tire of constantly writing checks like this for each parameter, you could check out one of the many open source .NET precondition enforcement libraries. I am fond of CuttingEdge.Conditions.

That way, you can use something like this:

Condition.Requires(list, "list").IsNotNull();

However, setting up a precondition like either of the above will just specify that your method does not accept nulls. Your problem will still exist in that you are passing nulls into the method! To fix that, you will have to examine what is calling your methods, and working out why null objects are being passed in.

1
votes

As well as throwing ArgumentNullException exceptions there's also something called the "Null Obejct Pattern" which you can use if you want to be passing around a null, to indicate, for example, that something doesn't exist, but don't want to have to explicitly check for nulls. Essentially it's a stub class that implements the same interface, but its methods are usually either empty or return just enough to make them comple. http://en.wikipedia.org/wiki/Null_Object_pattern

Also useful for value types that can't easily express their non existence, by not being able to be null.

0
votes

I would return early (or throw an InvalidArgumentException early) when given invalid input.

For example:

private void SetUserFriends(List<Friend> list) 
{ 
    if (list == null) 
        return;

    /* Do stuff */
}

Alternatively you can use the general null coalescing pattern:

private void SetUserFriends(List<Friend> list) 
{ 
    list = list ?? new List<Friend>();

    /* Do Stuff */
}
0
votes

You are indeed asking the wrong question. The right question is "does null represent an invalid input or a flag that means X".

Until non-null reference types are added to the language and make their way various API's, you have the choice of making that explicit in the code or letting null reference exceptions help you find where the expectation is violated and then fixing the data/code one way or another.