DEV Community

Cover image for Unhealthy Code: Null Checks Everywhere!

Unhealthy Code: Null Checks Everywhere!

James Hickey on September 05, 2019

This is an excerpt from my book Refactoring TypeScript: Keeping Your Code Healthy. Identifying The Problem Billion Dolla...
Collapse
 
moopet profile image
Ben Sinclair

I don't particularly like the NullBoss pattern.

It means you have to introduce fictional objects to work around a problem, which moves the problem elsewhere. Now you can't make a currentLevel.hasBoss function without making checks for your special object, or adding a property to it called isActuallyARealBoss.

In that sort of instance, I'd use an array, since there could be multiple bosses. As for being able to assign an empty array, I'd make sure I was using a language that allowed me to do that, because I value my mental health!

I agree that functions shouldn't generally return "thing or null".

Collapse
 
jamesmh profile image
James Hickey

One of the benefits of this pattern is that you typically would avoid the need for a hasBoss method (unless there's an exceptional case where you would need that check). Otherwise, using null objects should remove the need to do that.

I would agree that an array of bosses would work well, if the scenario warrants it.

Thanks! Great feedback.

Collapse
 
sambenskin profile image
Sam Benskin • Edited

What would the hasBoss function be used for? My understanding is the null object pattern would handle it for you. If you want current level to do something, then just do it and let the Boss or NullBoss handle what to do in each situation

Collapse
 
olvnikon profile image
Vladimir

This way of handling nullable objects reminds me a special type in Functional programming called Option (or Maybe). It keeps two values: None and Some(val). While Some always keeps a real not null value and you can map it, None has nothing but it is not null.
I would also apply this pattern instead of creating null versions of objects because it is more generic and covers all nullable cases. For example, fp-ts provides such utilities as fromNullable.

Thread Thread
 
jamesmh profile image
James Hickey

Agree. This was a solution I debated whether to include in the book or not. Due to not wanting to take forever with the book, I decided to exclude it.

But I agree, there are many situations where using the option pattern works very well 👍

Collapse
 
amatiasq profile image
A. Matías Quezada • Edited

I don't understand why do we need a whole class to create an array const myEmptyArray: string[] = EmptyArray.create<string>(); in C# you can just string[] myEmptyArray = new string[] {}; or List<string> myEmptyList = new List<string>();

Collapse
 
jamesmh profile image
James Hickey

Sure. There's a semantic preference and also, in some cases, a performance boost too.

In C#, for example, using Enumerable.Empty<T>() will use a singleton underneath the covers so you aren't allocating an empty array on the heap.

In very specific situations that could help. Otherwise, it's mostly a semantic preference.

Collapse
 
petergabriel2 profile image
PeterGabriel2

That enumerator was not good example. You can't compare mutable with immutable - in this case it is not about semantic.

Collapse
 
hegi profile image
Hegi

"What's wrong with that? Well, it allows your code (in this case, the Product class) to get into an inconsistent state."

So your real problem is that you are not following the basic rules of OOP. Your Product class should be responsible for not letting you create one with null ID in the first place.

And the saddest part is that neither of your examples above highlight the root cause / solution for the issue. I appreciate the gesture but unless this is not addressed, you are just treating a sympthom and being part of a much larger problem.

(The solution is correct, but it is applied at the wrong place)

Collapse
 
olvnikon profile image
Vladimir

What if a property or a property of a property is still optional? E.g., user.address.zipCode. There is a proposal to avoid intermediate checks. user.address?.zipCode. However, zipCode is still nullable. Having null/undefined in any place is code smell and potential hole in the application. That's why it is always better to have a not null result. The problem is persistent not only in OOP but in FP as well, however, FP has a good way to handle it. Instead of working with null it provides you None.

Collapse
 
jamesmh profile image
James Hickey

I would disagree. Following the "basic rules of OOP" is not in-and-of-itself an end. It is a means to an end, just like any programming paradigm. Who cares if you follow OOP?

What's the end? To have software that is easy to understand and therefore can be maintained easily, deliver business value, etc.

Also, there's nothing in OOP that says a class' property cannot be null.

The article was about treating issues around nulls and the fact that TypeScript allows for null variables. That applies whether you are doing OOP or not, using classes or not, etc.

Thanks for the comments! 😊

Collapse
 
hegi profile image
Hegi

Thank you for proving my point. Much appreciated

Collapse
 
alexdevpro profile image
Alexander

I agree with Hegi. Just add a null check once in the class constructor and you won’t need to do a check in all parts of the code that work with an instance of this class.

Thread Thread
 
jamesmh profile image
James Hickey

I disagree with the notion that "just adding a null check in the constructor" is always the best solution.

The fact is that any solution you choose is going to have trade-offs.

I've written about using constructor validation here and I agree that there are some contexts when using this technique is the best choice.

That being said, there are trade-offs to using constructor validation:

Mainly: You have to throw an exception to notify the caller that they did something wrong.

The caller (and developer using your class) has to have implicit knowledge up-front that your class will throw when used incorrectly. That can lead to developers potentially using your class wrong.

This also depends on whether your code is a publically accessible library or custom/internal business logic for a company.

With a public library, you aren't guaranteed to be able to educate users of the fact that your class will throw (other than in documentation, them finding out "the hard way", etc.)

What if that person wants to use your library in a performance-sensitive context? Throwing exceptions has a huge performance impact... so they may not opt for your library now.

The same applies to custom business code: does it need to be used in a performance-sensitive context? If so, throwing exceptions is the first offender to that most devs will look at removing.

Now, looking at the technique in this article, it allows the caller to not have to know about nulls and having to check them. Sure, it's a different technique, but now you have hidden/encapsulated the need to check for nulls, it's more performant since you don't need to catch exceptions, etc.

Is it a bit more complicated (internally)? Sure. But that's the price you pay in order to make code that's easier to use or has less of an entry barrier.

SOOO... which way is best?

Well, it depends! And that comes back to my comments with @hegi - just saying that one technique works best in all contexts across the board leads to issues when perhaps the context in question is something like a performance-sensitive, etc.

This is the same thinking people have when they say that "microservices is the best architecture and should always use it." That attitude and approach can (and has) destroy companies when used in the wrong context.

Thanks for your thoughts @alexander !

Collapse
 
dimpiax profile image
Dmytro Pylypenko • Edited

So instead of making guards using null, you propose to create useless empty array and objects, right? Just to spam memory with a empty values of nothing?

As I am seeing from the comments you propose to use NullObject pattern, like everywhere, to fill any of gaps, which makes the code leaking to any executing instead of breaking the flow when it needs.

Collapse
 
kopseng profile image
Carl-Erik Kopseng • Edited

"makes the code leaking to any executing".

What?

"instead of making guards using null, you propose to create useless empty array and objects"

No. Instead of making 10 null checks, you do 1.This pattern never makes sense if you just want to avoid a single null check. But when you do the same checking at multiple levels, this vastly simplifies logic at times.

Collapse
 
dimpiax profile image
Dmytro Pylypenko

Review your comment in 10 years.

Thread Thread
 
kopseng profile image
Carl-Erik Kopseng

Immutable code where you can drop all null checking is fantastic to work with. And I have been using the Null object (/DefaultObject pattern) for a decade in various settings. Still does wonders for avoiding nasty NPEs in Java and works quite well in JS as well, though I am pragmatic: I do not create a full 1:1 mapping for every class. Just the ones where it makes sense.

Thread Thread
 
dimpiax profile image
Dmytro Pylypenko

Hi, the talk in my comments is not about mutability , but about clean and not overused code with default values where execution must be terminated, but not being executed with empty values.

Collapse
 
dontry profile image
Don

Hi James, how do you instantiate the NullBoss class? I reckon it still needs to do the null check in the first place, doesn't it?

Collapse
 
jamesmh profile image
James Hickey

The NullBoss doesn't do any null checking because it replaces the concept of null altogether.

const nullBoss = new NullBoss();
nullBoss.fight(player);
// Etc.

Usually with kind of pattern, you would have a factory of some kind create your objects for you.

Does that help?

Collapse
 
dimpiax profile image
Dmytro Pylypenko

Could you provide the practical example with NullBoss?
Cause the current example is not like dummy date, that can be used as mock. But the example which is far from reality, correct me if I wrong.

Thread Thread
 
olvnikon profile image
Vladimir

I think the whole idea of using NullObject is taken from Option type, which holds None or Some. When they is no null you don't think it exists. E.g. while Some holds a real not null value, which can be mapped, None always returns None. This approach is called "Railway programming".

fromNullable(null).map(double).map(tripple); // None
fromNullable(2).map(double).map(tripple); // Some(12)

null is avoided and the program doesn't crash. Just different way of thinking when there is no null.

Thread Thread
 
dimpiax profile image
Dmytro Pylypenko

As I know, the pattern was a long time before Optionals (some, none).
And what author proposes – it’s a big difference from it.
Using optionals you stop the execution flow, while the topic about redundant objects creation.

Thread Thread
 
lukejs profile image
Luke

2 examples I've seen in Minecraft:

Air blocks - they are blocks like any other, rather than nulls to represent a block not being present in a location

Also added in the last few versions (replacing null) - "Empty" item stacks - there is a singleton "ItemStack.EMPTY" and a method "ItemStack#isEmpty()"

Collapse
 
kopseng profile image
Carl-Erik Kopseng

Like in all OOP, you still need to make the decision on which class to create somewhere. What you gain by taking the decision at a higher-up place is the ability to avoid this decision in all other places.

Collapse
 
pencillr profile image
Richard Lenkovits

This came as divine intervention. I'm currently struggling with a null check hell case because of some inconsistently structured api data, and I was thinking about what would be the clean way to deal with it. Then this article came up. Thanks! :)

Collapse
 
ifarmgolems profile image
Patrik Jajcay

If you're using JS, take a look at lodash.com/docs/#get

Collapse
 
jamesmh profile image
James Hickey

Sweet! The book covers the special case pattern also.

Collapse
 
bendem profile image
bendem • Edited

Sometimes null is useful but generally, it plagues the code. Null being part of the type system like typescript allows is really neat (props to Kotlin, Rust and C++ too). Having the compiler yell at you for checking for null when something can never be null makes code a lot more readable without leaving the writer wondering whether this or that will actually be null or not.

Nice article.

This part though, I don't understand:

Take 2
Other languages like C#, Java, etc. won't allow you to assign a mere empty array to a collection due to rules around strong typing (i.e. []).

In those cases, you can use something like this version of the Null Object Pattern:

class EmptyArray<T> {
    static create<T>() {
        return new Array<T>()
    }
}

// Use it like this:
const myEmptyArray: string[] = EmptyArray.create<string>();

I'm not sure what this is supposed to mean. You can make empty arrays in C# and Java just fine (though you generally use an empty collection instead of raw arrays).

String[] myEmptyArray = new String[0];

// Even better
Collection<String> myEmptyCollection = List.of();
// Or in older versions
Collection<String> myEmptyCollection = Collections.emptyList();
// Or in even older versions
Collection myEmptyCollection = Collections.EMPTY_LIST;

The code you provided just does nothing, I'm not sure what problem you tried to solve by writing it.

Collapse
 
helenanders26 profile image
Helen Anderson

Great post!

I've just been reading about "The Billion Dollar mistake" from a SQL point of view, interesting to hear another perspective on the same.

Collapse
 
jamesmh profile image
James Hickey

Oh ya... NULL in SQL can be even trickier!

Collapse
 
mchmielarski profile image
Maciej

nice post! but one typo in examples

const myEmptyArray: string[] = EmptyArray.create<T>()

should be

const myEmptyArray: string[] = EmptyArray.create<string>()
Collapse
 
jamesmh profile image
James Hickey

Good catch! Thanks

Collapse
 
cfv1984 profile image
CFV • Edited

Optional types are where it's at, and there's some prebuilt ones that are more or less free to use in terms of complexity.

Rearchitecting your entire application to deny the fact that nulls exist as a valid value strikes me as basically the nuclear option when compared to dealing with them head on.

Collapse
 
kopseng profile image
Carl-Erik Kopseng

It is not necessarily about nulls, but what to do in the absence of a value. You can often find multiple types of these null classes to cover different cases. And it does mean you have to rearchitect anything. For instance, when designing a Card component in a React application, that was dealt a User object, we assumed it would always exist. Suddenly, it was potentially optional, but that would imply changing user: User to user: User | undefined through multiple levels. Simply creating a NullUser that was returned at the top level selector function allowed us to make 0 (zero) changes further down.

Collapse
 
ronancodes profile image
Ronan Connolly 🛠 • Edited

What about using undefined instead of null?
Better or worse?

Collapse
 
emptyother profile image
emptyother

Undefined in javascript is what null is in other languages. I think its better to use it. Best is to avoid both.

But a lot of js developers expect a null. A lot of libs return null (some even return either, with no reason for one or the other). And js itself also returns null, for example getElementById.

Collapse
 
ronancodes profile image
Ronan Connolly 🛠

The reason I use it is that if you see a null you known that it was explicitly set, where as undefined means it hasn't been touched.

Would you recommend just sticking to undefined rather than mixing null and undefined?

Thread Thread
 
emptyother profile image
emptyother

Do your code ever do things differently based on if a variable contains a null or if it contains a undefined?

Let me try a bad analogy: If couldYouBuyMeSomeFlowers() returns null, it would be reasonable to assume that the florist is out of flowers (why else would he give us an explicit null instead of nothing).. But if it returns undefined.. Did the method forget to go to the florist? Should i call couldYouBuyMeSomeFlowers() again until it returns flowers or null? Or is the florist gone? No matter the reason, I still don't have flowers and I will have to throw a PartyCanceledError("Sorry, we can't have a party without flowers").

If i needed a reason why the method failed, it would be better if the method just threw an error instead. StoreNotFoundError or NotEnoughMoneyError. Or a Promise<Flower[]> for a later delivery. I could deal with that.

Collapse
 
joelvarty profile image
Joel Varty

Nicely written - a big part of this comes from creating a pattern and following it closely, especially with a team. Huge value in having code that's not only clean but easily predictable.

Collapse
 
jamesmh profile image
James Hickey

👍 Def. Having some clear standards, common patterns, etc. will help a team be on the same page. Codebases where there's no predictability or conventions are hard to maintain, etc. 😥

Collapse
 
joelvarty profile image
Joel Varty

💯

Collapse
 
ankitasinghal profile image
Ankita Singhal

This is a nice article. Cool tips. I do see a lot of code with checks for null/undefined.

I myself am guilty of doing it sometimes. I will be more mindful after reading this in doing null checks :)

Collapse
 
jamesmh profile image
James Hickey

Great! Yes, it's very common :)

Collapse
 
devdufutur profile image
Rudy Nappée

Or just use multi level object destructuring with default values, it's much simpler !

Collapse
 
kopseng profile image
Carl-Erik Kopseng

Often not an option. You also assume there are no methods to call or behaviour to deal with.

Collapse
 
mburszley profile image
Maximilian Burszley