What's wrong with returning null?

Question

I've recenlty been greeted by CS8603 - Possible null reference return, which indicates that my code could possibly return null. It's a simple function that looks up an entity in a database by id - if it exists, it returns the entity. If not, it returns null.

public TEntity Get(Guid id)
{
    // Returns a TEntity on find, null on a miss
    return _entities.Find(id);
}

My gut feeling says that this makes sense. If the client says "Give me the user with ID 82739879", then giving them "nothing" makes intuitive sense, if no user with said ID exists".

However, the compiler warning caused me to re-think this approach. Should I do something else, other than returning null, if the user ID could not be found? Returning an exception is a possibility, but I don't consider a user not existing to be an "exceptional" state.

Is it "wrong" to return null in this case? Am I overthinking this?

David Arno · Accepted Answer · 2021-11-09T09:10:58.427

Why are you getting the warning?

You have enabled the nullable reference types (NRT) feature of C#. This requires you to explicitly specify when a null may be returned. So change the signature to:

public TEntity? Get(Guid id)
{
    // Returns a TEntity on find, null on a miss
    return _entities.Find(id);
}

And the warning will go away.

What is the use of NRTs?

Other recent changes - specifically around pattern matching - then tie in really nicely with NRT's. In the past, the way to implement the "try get pattern" in C# was to use:

public bool TryGet(Guid id, out TEntity entity)

Functional languages offer a better approach to this: the maybe (or option) type, which is a discriminated union (DU) of some value and none. Whilst C# doesn't yet support DU's, NRT's effectively provide that maybe type (or a poor man's equivalent) as TEntity? is functionally equivalent to Maybe<TEntity>:

if (Get(someId) is TEntity entity)
{
    // do something with entity as it's guaranteed not null here
}
else
{
    // handle the fact that no value was returned
}

Whilst you can use this type of pattern matching without using NRTs, the latter assists other developers as it makes clear that the method will return null to indicate no value. Change the name to TryGet and C# now provides that functional style try get pattern:

public TEntity? TryGet(Guid id) => _entities.Find(id);

And with the new match expression, we can avoid out parameters, mutating values etc and have a truly functional way of trying to get an entity and creating one if it doesn't exist:

var entity = TryGet(someId) switch {
    TEntity e => e,
    _ => Create(someId)
};

But is it wrong to return null?

There has been vast amounts written on why null was the billion dollar mistake. As a very crude rule of thumb, the existence of null likely indicates a bug. But it's only a crude rule of thumb as there are legitimate use-cases for null in the absence of Maybe<T>. NRT's bridge that gap: they provide a relatively safe way of using null to indicate no value. So I'd suggest - for those using newer versions of C# - there is nothing wrong with returning null as long as you enable the NRT feature and you stay on top of those CS8603 warnings. Enable "treat warnings as errors" and you definitely will stay on top of them.

Heinzi · Answer 2 · 2021-11-09T18:50:57.320

David Arno already answered your question about the specific warning, so I'd like to address your general question:

What's wrong with returning null?

Nothing, as long as the consumer of your method is aware that null might be returned, and, thus, it is their responsibility to react appropriately.

If your language supports null annotations: Use them. If it doesn't (e.g., if you are stuck with a classic .NET Framework 4.8 project), my go-to solution is to name the method appropriately, i.e., GetOrNull, GetIfExists, etc. Often, I have both (Get throwing an exception and GetOrNull returning null), if I need to cover both use cases:

public TEntity GetOrNull(Guid id) => _entities.Find(id);
public TEntity Get(Guid id) => 
    GetOrNull(id) ?? throw new ArgumentException($"Entity {id} not found.");

score 5 · Answer 3 · answered Nov 09 '21 at 02:36

So as per This answer, you can make the warning go away by explicitly specifying when a null might be returned.

Think of the caller however - they're still going to need to do a check for null.

I prefer using the Try Get pattern. You return a boolean, with the actual return value you were after as an out parameter, like so:

public bool TryGet(Guid id, out TEntity entity)
{
    entity = _entities.Find(id);
return entity != null;

}

For the caller, this can be quite a bit more convenient. It makes it very easy to use a guard pattern such as the following:

TEntity entity;
if(!TryGet(id: idIWantRecordFor, out entity))
{
    entity = new TEntity
    {
        Id = Guid.NewGuid(),
        SomeProperty = "Some content"
    };
}
// at this point, no matter what, we've got a usable TEntity
// either one we got from the db, or a default, or a new one, etc.
Console.WriteLine(entity.SomeProperty);

gnasher729 · Answer 4 · 2021-11-09T14:13:24.953

There is nothing wrong with null pointers in principle.

What's wrong with older languages is that there is no way to know whether a pointer could be null or not. So you have to check whether it's null, even though according to your code it never can be null. Wasting code and time. Of course the one time where you don't check it's Null. The other extreme is when data can be present or absent and you have a language that doesn't allow a pointer to be non-null. Then you have to produce a pointer with fake data if the data is absent, just as bad.

Newer languages have "optional" values. In Swift, for every type T there is a type optional<T>. So your TEntity CANNOT be null. But an optional<TEntity> can be null. Normal code doesn't allow you to try to access the data in an optional<TEntity>. There is a special "if" that lets you try to extract the TEntity, while at the same time reporting success or failure, and if extracting the TEntity failed, then it is not accessible.

The language doesn't allow unnecessary tests - if you have a variable of type TEntity it CANNOT be null, and the compiler doesn't allow you to test if it is null. If it is of type optional<TEntity> then you MUST test if it is null in some way. So you get 100% safety, And you can very easily report that some value isn't present.

if let result = something.get(guid: someGuid) {
    // result is a valid TEntity
} else {
    // There is no TEntity, and no variable "result"
}

score 0 · Answer 5 · answered Nov 27 '21 at 00:59

What does it mean to get a record by id when that record doesn’t exist? That sounds like an error to me. I have tons of code that tries to get a record by id and then throws if the record is null, because that is the record I am supposed to be displaying or changing, and if it doesn’t exist there’s nothing else sensible I can do.

Throwing an exception is what you should do when you don’t have a way to do what you are supposed to do. Your signature says you are supposed to return a TEntity, are you returning a TEntity? No, you are sometimes returning a null value. Either your signature doesn’t say what you want it to, or you should throw.

score -1 · Answer 6 · answered Nov 11 '21 at 00:58

"In the days of yore," functions indicated error-conditions by returning particular values. Unfortunately, this put the burden upon every direct or indirect caller of those functions to "always do the right thing."

As time went on, we realized the value of "exceptions."

So: When you realize that "no user exists," instead of indicating this by a return-value (such as NULL ...) you treat it as "an exception." Instead of "returning" anything, instead you create an exception-object and throw it up into the air ... expecting someone to "catch" it.

In practice, this works quite well: "ordinary" program logic is able to proceed, simply assuming that "if I am still here, nothing went wrong." Exceptional cases are moved to an entirely different path which consists of "throwers" and "catchers."

Any point in the program which detects an exceptional condition can "throw." Meanwhile, the "catchers" don't have to care where the "throw" actually came from.

score -1 · Answer 7 · edited Dec 18 '21 at 14:54

-1

Just do this, adding ? Example:

public async Task<TblEstudiante?> GetAsynEstuByID(int id)

That's all

edited Dec 18 '21 at 14:54

lennon310

3,242

answered Dec 18 '21 at 13:20

Easy To Make

9

score -4 · Answer 8 · answered Nov 09 '21 at 07:19

There is absolutely nothing wrong with returning null if the business case says the data is optional. I worked as a senior Java developer in large enterprise for 10 years, and had my share of null pointer exceptions to fix. After a while I realized that the majority of the exceptions indicated there was business use case that we didn't handle correctly, if at all. For instance our mainframe database would specify that customer date of birth was a mandatory field and would never be null. But it turns out we did not collect this field for the first several years, so some records would have null value. You can't fix this without revisiting the business use case - what should we do with ancient customer data? Hiding the exception with Optional and the like fixes the technical problem but creates a subtle bug. Long live the NPE!

What's wrong with returning null?

8 Answers8