At work, we’re full steam ahead with .NET Core 3 and, thusly, nullable reference types. And we’ve come to realize that we need some kind of policy to dictate how nullable reference types will affect our code. It’s not just a matter of turning the feature on — you have to make careful decisions about where ? should be used in the codebase.
So, I wrote the first draft of such a policy and sent it to my teammates.
The policy
The most important rule, and why
Any time you know that an external system could pass a null reference object to a function, you should declare that parameter as nullable (?).
In the following example, the compiler will not warn you to check dto for null in ControllerAction because RepositoryMethod accepts a null dto. In other words, RepositoryMethod is telling the compiler it’s okay if it gets a null dto.
public void ControllerAction(Dto? dto) {
RepositoryMethod(dto);
}
private void RepositoryMethod(Dto? dto) {
}If RepositoryMethod does not accept a null dto, as in the following example, the compiler will warn when you pass the dto:
public void ControllerAction(Dto? dto) {
RepositoryMethod(dto); // WARNING! Check for null first!!!
}
private void RepositoryMethod(Dto dto) {
}Just to clarify a bit more, the compiler complains any time you access a nullable reference type before checking for null, so you’ll get a warning if you do something like this:
public void ControllerAction(Dto? dto) {
dto.Something(); // WARNING! Check for null first!!!
}Again, we can’t prevent external systems from passing null to ControllerAction so dto should be nullable. Since repositories are internal systems that are only called from code within our solution, we can rely on the repository to decide if null should be allowed.
That has the potential to push some null checks further down into the stack, but that’s a good thing, I think. We’re allowing null objects to be passed further down the stack where low-level components can decide if null is valid.
That being said, a repository method probably doesn’t have any reason to allow null objects. That leaves it up to the controller to check for null, and there’s no reason to check for null in the repository method (as long as you’re paying attention to the compiler warnings).
Kickback
Of course, I walked away thinking I had just written the epitome of null reference object policies. But not everyone felt the same.
Queue heated developer debate.
His feeling was that high-level methods should check for null regardless of what the compiler says. A high-level method called Create should immediately and always check for null regardless of what the code in the method allows. Logically, I get it. 99.9999% of the time, it doesn’t make sense to pass null anything to a method called Create.
My thought is that it doesn’t matter what the method is called. If a high-level method preemptively checks for null, you end up with conditions like the one below where a branch of code never gets called:
public void Create(Dto? dto) {
if (dto == null) throw new ArgumentNullException(dto);
RepositoryStuff(dto);
}
private void RepositoryStuff(Dto? dto) {
if (dto == null) {
// This code will never be called
}
}He doesn’t think it’s right to allow low-level code to make that determination, even if it says it will by allowing a nullable reference object.
It’s not so much that he doesn’t trust the low-level code to do the null check — although I suspect that’s the case — It’s that, fundamentally, it doesn’t make sense for a Create method to accept null at all.
As sometimes happens, we agreed to disagree.
What say you?
I’m kind of at a loss. I started writing this post before all the debate started, but now I’m finishing it to get some feedback. I’m curious what you think about this situation and what kind of policy you have in place for null reference objects.
