Returning null or what?

Question

I have a C# function like below, and in the else block, the function must return something. I don't know whether returning null is a good practice or not.

public Nullable<Point> GetSprite(int x, int y)
{
    if (x <= xCount && y <= yCount)
    {
        return new Point(x * spriteHeight, y * spriteWidth);
    }
    else
    {
        return null;
    }
}

Answer 1

Yes, you could just write:

public Point GetSprite(int x, int y)
{
    if (x <= xCount && y <= xCount)
    {
        return new Point(x * spriteHeight, y * spriteWidth);
    }

    return null;
}

And write in the documentation it could return null if if (x <= xCount && y <= xCount)

So who use your code could just write

Point point = GetSprite(x, y);
if (point == null) { /* something went wrong, check variables! */ }
else { /* ok! go ahead! */ }

I found this syntax very useful, and could not lead to bugs if the programmer is alerted about the null.

And, anyway you don't need the else block since the return call will stop the function (except if there are any finally blocks, but it's not the case.).

Answer 2

Null is bad. It's a flaw in the type system, and its inventor deeply regrets unleashing it upon the world. It adds an additional value to all reference types that you probably don't want.

If you don't want to throw an exception (because failing to return a Sprite is a common occurrence and will likely be handled immediately rather than several layers up), then the correct solution is to return a type that represents a value of type T that may or may not be there. For value types, Nullable fills that need reasonably well, because it forces you to handle the case where the value is missing. For reference types you'll have to roll your own since C# doesn't provide any.

Answer 3

It depends whether arguments that are out of range may occur in a regular way or if such method calls are considered being abusive.

• If such arguments are regular, then return null (that's perfectly okay).

• If such arguments are abusive, then throw an exception.

An exception should always catch an error and never be misused for a test.

If the purpose of the method is not only to return a sprite, but also to test whether a sprite is available for a set of parameters, then you could use the same pattern as with Dictionary<T>.TryGetValue:

public bool TryGetSprite(int x, int y, out Sprite sprite)
{
    ...
}

Use it like this:

Sprite sprite;
if (obj.TryGetSprite(x, y, out sprite)) {
    // Sprite available
} else {
    // No sprite available
}

When the return type is a non-nullable type, this is a good option. Magic numbers like 0, -1 or Int32.MinValue are not obvious and are not always available (if the full range of values is required as a regular result). Even null might be a regular result under certain circumstances; for instance, a dictionary is allowed to contain null values. If you need this distinction between contains null and not found, the TryGet... approach is perfect.

The advantage over returning null is that you don't need additional knowledge apart from the method's signature in order to know how it works. Otherwise you are not sure whether an exception will be thrown or whether null or even an empty Sprite will be returned without reading the documentation. The disadvantage of TryGetSomething is that is looks somewhat cumbersome. However, starting with C#7.0 / Visual Studio 2017, you can declare the variable inline: obj.TryGetSprite(x, y, out var sprite) or obj.TryGetSprite(x, y, out Sprite sprite).

Answer 4

I've read alot about returning null is not the best practice in the world, but like anything I think it will depend on it's usage and how often it might be called. So, in your case this might be ok. However, I'd just like to chuck out another way of doing things using the Null Object pattern.

Using this you would not return null at all but rather return a Null/invalid object. Your calling code dependant on it's usage will be checking an instance type and so you might not need to even worry about whether it's null and potentially remove all those == null checks.

public Point GetSprite(int x, int y)
{
    return x <= xCount && y <= xCount
        ? new Point(x * spriteHeight, y * spriteWidth);
        : Point.OffGrid();
}

Then in your Point class itself you might have something like

public class Point
{
    private static readonly Point _null = new Point(-9999,-9999);

    public static Point OffGrid
    {
        get
        {
            return _null;
        }
    }

    public bool IsOffGrid {
        get { return this.Equals(OffGrid); }
    }
}

This way you might never even need to check for null at times. However if you did your statements might look something like this now

var point = GetSprite(x, y);

if(point.IsOffGrid()) // equivalent to point == null
{
   // how do we handle it when it's off grid?
}

// do stuff

Answer 5

Returning null is fine. You can check for null and decide what to do in the calling function.

Multiple return statement in a function is not a good practise.

So you can modify your function like:

public Point GetSprite(int x, int y)
{
    Point pObj = null;
    if (x <= xCount && y <= xCount)
    {
        pObj = new Point(x * spriteHeight, y * spriteWidth);
    }

    return pObj ;
}