What to do when a function to return something is null?

Question

I have a generic Items class that keeps track of all items, and when Items are created they are added to this list. The problem is that I have a function that returns the item by its ID, but what do I return in items[id] doesn't exist? It won't let me return null...

Should I just use a for loop that iterates through every item and checks if items[i].id equals the input ID?

Here's my code

public static class Items
{
    public static List<Item> items = new List<Item>();

    public static Item getItemById(Items instance, int id)
    {
        if (items[id] != null)
        {
            return items[id];
        }
        else
        {
            return null;
        }
    }
}

Answer 1

Not sure what your problem is. you use the id like a position in an array.

try to replace

if (items[id] != null)

with

if (items.Count > id)

this way if the id position doesn't exist in the array the function will return null value and the program won't crash.

If you want to use the ID like an ID of an object in a more properly way check the solution given by Spectre.

Answer 2

I recommend the short syntax:

public static Item GetItemById(Items instance, int id)
{
    return (items.Count > id) ? items[id] : null;
}

Or even shorter:

public static Item GetItemById(int id) => (items.Count > id) ? items[id] : null;

By the way:
In C# method names should begin with an uppercase letter.
And do you really need the Items instance parameter?

Answer 3

 It won't let me return null...

It sounds then like your Item type is a struct (ie. a value type)

Reference types (including anything defined as a class) are stored and passed by reference (ie. if you have a GameObject myObject variable that variable holds a kind of pointer to the game object, rather than replicating the whole object inside the variable's storage), and that reference can point at null to signify "nothing / not populated / invalid"

But value types (like numbers, or anything defined as struct) are stored and passed by value (by default). So each such variable contains a complete and independent copy of the data. Eg. an int variable will contain a specific four bytes, and every possible value of those bytes represents a valid integer from int.MinValue to int.MaxValue. So there's no leftover storage to represent the "nothing / not populated / invalid" message that null provides for reference types.

If you're used to working with reference types, you can just change Item to be a class and you're off to the races. This can save memory in some cases because we can share references to a common instance rather than copying it everywhere, but it also means we're doing heap allocations whenever we create a new instance, which might not be ideal for types you want to create & discard frequently, like vectors.

When we need to, we can augment a value type with an extra bit of storage to track whether it's been populated, by using a Nullable type. This is a built-in generic struct that wraps an arbitrary value type and adds null functionality.

We can declare that our method returns a nullable version of the Item type (ie. a Nullable<Item>) by writing:

 public Item? GetItemById(int id) { ... }

This will let us return an Item instance or null, and the ? advertises to callers "I might fail to find an Item for you — I can only promise a 'maybe'"

Calling code should then check to see if it got a null before it tries to work with the returned value:

Item? response = myItems.GetItemById(someId);

// Check if we found an item at all.
if(response.HasValue) {
    // if(response != null) also works here

    // Get the actual item value.
    Item item = response.Value;

    // Do something with it...
}

Answer 4

Should I just use a for loop that iterates through every item and checks if items[i].id equals the input id?

Yes, you should I've edited your code a bit.

public static class Items
{
    public static List<Item> items = new List<Item>();

    public static Item getItemById(int id)
    {

        foreach(Item it in items) // iterate through items
            if(it.ID == id) return it; // if one of the items has provided ID, return it

        return null; // if none of the items has the id, return null

    }

}

Also, make sure your Item class has an ID property:

public class Item {
    public int ID { get; set; }

    // more logic
}

Answer 5

null references are dangerous. Every null lurking in your data is a landmine waiting to explode in a NullReferenceException. These can be hard to debug, because in a more complex program it might not always be obvious when, where and why a reference was set to null.

Consider throwing an Exception instead. Create an own exception class UnknownItemIdException and throw that exception instead of returning null.

public static Item getItemById(Items instance, int id)
{
    if (items[id] != null)
    {
        return items[id];
    }
    else
    {
        throw new UnknownItemIdException(id);
    }

}

Answer 6

So there's a few questions here. What do i return? what if it doesn't exist? should I use a for loop?

Firstly. A couple of good practises when attempting to find something which potentialy cannot be found.

1) Name the function with 'find' instead of 'get' as it implies the function behaviour. 'get' kind of implies that item exists and so the function should just go go and 'get' it. However 'find' implies some work may have to be done. A search maybe. Also the find may not be successful. This is useful when working with a team of programmers as they may not be intimately aware of what your code does. The programmer can read the function name and have an idea of whats going on.

2) The find may fail. This is a valid scenario. Write the code accordingly. Return false if the item is not found. On success output the found item as an output parameter.

So an example which may help your situation may be..

    static void Main(string[] args)
    {
        Item[] allItems = new Item[100];

        // make a test item with id of 45 and add it to the list
        Item testItem = new Item(45);
        allItems[testItem.ID] = testItem;

        // find it
        Item foundItem;

        if (findItemById(allItems, 45, out foundItem) == true)
        {
            // Item was found !
        }
    }
    static bool findItemById(Item[] itemStore, int id, out Item foundItem)
    {
        if( itemStore[id] != null )
        {
            foundItem = itemStore[id];
            return true;
        }

        foundItem = null;
        return false;
    }

secondly, Should you use a loop to iterate and check?

I would say yes but the reason why and not how is probably more important.

To write the code ..

if (findItemById(allItems, 45, out foundItem) == true)

This is pretty straight forward. true on success. false on fail. founditem is set if success. You can write this once at the start of the project and it can easily be there at the end with no modifications.

However the findItemById() function offers a simple interface and can hide any implementation intricacies. So you can alter and optimise what it does throughout development. So if you use a list, and check for id's, if the list gets long and this function shows up on the profiler then you can hone in your optimisation time.

hope that helps :)