1

What is the correct way to use values that may or may not have the value of null?

I have this piece of code that sends an email to a manager if the employee who is taking a holiday request has a manager. 

 if(holidayRequestForm.Employee.Site.SiteManagerEmail != null)
            { 
            SendMailToManager();
            }

However this is causing a NullReferenceException.

What would be the correct way to implement calling the SendMailToManager() if that Employee has one without causing a NullReferenceException.

Is it bad practise to use possible null values this way?

Conor8630
  • 345
  • 1
  • 17
  • You are assuming here that `holidayRequestForm`, `Employee`, and `Site` all exist. The null reference exception will be because one of those is null and not the last property. – Craig H Jun 10 '19 at 11:03
  • @CraigH I have validation in place that all employees must be assigned a Site, so that should avoid them being null – Conor8630 Jun 10 '19 at 11:06
  • How is ASP.NET MVC related to the question? It looks like any of those variables or properties could be null and result in an NRE. Having validations doesn't mean the properties *are* filled. Perhaps an EF query forgot to load the related entities. Perhaps the properties weren't assigned while loading, or perhaps something *set* them to null explicitly. – Panagiotis Kanavos Jun 10 '19 at 11:07
  • You should check the exception's call stack. Right now we don't know if the exception was thrown by one of the properties or `SendMailToManager()`. The duplicate question shows how you can debug such issues. – Panagiotis Kanavos Jun 10 '19 at 11:12
  • @PanagiotisKanavos It's the framework I am using, I thought the model might have being important. Could you explain why having validations doesn't mean the properties are filled? – Conor8630 Jun 10 '19 at 11:12
  • @Conor8630 if you don't load the values from the database, you'll have nulls. If the data comes from a form that doesn't fill anything, you'll also have nulls. Obviously, your code didn't load everything you thought it would Using `?.` can avoid nulls but won't tell you *which* property is `null`. This is probably not a problem if you don't really need those properties – Panagiotis Kanavos Jun 10 '19 at 11:14
  • Okay that makes sense. I do think the data is not being loaded and I think I know why (i think it might be in the `SendMailToManager()` itself.), thanks for the help, I appreciate it. – Conor8630 Jun 10 '19 at 11:20

1 Answers1

5

Take a look at Null-conditional operator ?. in the docs in order to understand how you can rewrite your if statement (avoiding NullReference exceptions).

if(holdayRequestForm?.Employee?.Site?.SiteManagerEmail != null)
{
    SendMailToManager();
}

I don't find it a bad practice. I do not think you have any other possibilities in the described situation.

Florin-Constantin Ciubotariu
  • 3,737
  • 4
  • 25
  • 33
  • 1
    Might be good to know, C# 8.0 helps avoiding this kind of problems by introducing nullable reference types. More on this here: https://learn.microsoft.com/en-us/dotnet/csharp/tutorials/nullable-reference-types – johey Jun 10 '19 at 11:09
  • @johey this won't prevent *external* nulls. In this case it looks like the data is loaded from the database. Null checks are still needed in this case. – Panagiotis Kanavos Jun 10 '19 at 11:11