DEV Community

Cover image for Code Smell 149 - Optional Chaining
Maxi Contieri
Maxi Contieri

Posted on • Originally published at maximilianocontieri.com

Code Smell 149 - Optional Chaining

Our code is more robust and legible. But we hide NULL under the rug

TL;DR: Avoid Nulls and undefined. If you avoid them you will never need Optionals.

Problems

Solutions

  1. Remove nulls

  2. Deal with undefined

Context

Optional Chaining, Optionals, Coalescence, and many other solutions help us deal with the infamous nulls.

There's no need to use them once our code is mature, robust, and without nulls.

Sample Code

Wrong

const user = {
  name: 'Hacker'
};

if (user?.credentials?.notExpired) {
  user.login();
}

user.functionDefinedOrNot?.();

// Seems compact but it is hacky and has lots
// of potential NULLs and Undefined
Enter fullscreen mode Exit fullscreen mode

Right

function login() {}

const user = {
  name: 'Hacker',
  credentials: { expired: false }
};

if (!user.credentials.expired) {
  login();
}

// Also compact 
// User is a real user or a polymorphic NullUser
// Credentials are always defined.
// Can be an instance of InvalidCredentials
// Assuming we eliminated nulls from our code

if (user.functionDefinedOrNot !== undefined) {  
    functionDefinedOrNot();
}

// This is also wrong.
// Explicit undefined checks are yet another code smell
Enter fullscreen mode Exit fullscreen mode

Detection

[X] Automatic

This is a Language Feature.

We can detect it and remove it.

Tags

  • Null

Conclusion

Many developers feel safe polluting the code with null dealing.

In fact, this is safes than not treating NULLs at all.

Nullish Values, Truthy and Falsy are also code smells.

We need to aim higher and make cleaner code.

The good: remove all nulls from your code

The bad: use optional chaining

The ugly: not treating nulls at all

Relations

More Info

WAT?

Credits

Photo by engin akyurt on Unsplash


He who fights with monsters might take care lest he thereby become a monster. And if you gaze for long into an abyss, the abyss gazes also into you.

Nietzsche


This article is part of the CodeSmell Series.

Top comments (29)

Collapse
 
loucyx profile image
Lou Cyx

This is your opinion, and is ok ... my problem is mainly that is presented as fact, when it isn't.

// I agree that we should avoid "falsy" values, so for that you can just combine
// optional chaining with nullish coalescing and default to `false` if the value
// is `undefined`, making the value always an actual boolean.
if (user?.credentials?.notExpired ?? false) {
    user.login();
}

// This is also ok, if the function exists you call it, if it doesn't, you don't.
user.functionDefinedOrNot?.();

// If the function returned a value, then ideally you should use nullish
// coalescing again.
const value = user.functionDefinedOrNot?.() ?? "default";
Enter fullscreen mode Exit fullscreen mode

You say that it "seems compact", but it IS compact. You also say is "hacky", but...

// This (the old way of "solving" the problem):
if (user && user.credentials && user.credentials.notExpired) {
    user.login();
}

// If way more "hackier" than this:
if (user?.credentials?.notExpired ?? false) {
    user.login();
}

// And doing this:
if (user.functionDefinedOrNot !== undefined) {
    user.functionDefinedOrNot();
}

// Is EXACTLY the same as doing this:
user.functionDefinedOrNot?.();
Enter fullscreen mode Exit fullscreen mode

Your proposed "solutions" are to either take more memory with default values (in the case of the object), or do an unnecessary checks that are already done for you with optional chaining (in the case of the function). You can see how TypeScript handles optional chaining when compiled as ES5 here, but the long story short is that is pretty much the same as what you did with the if, but with less boilerplate.

Now don't get me wrong, I agree with your point about how null should be avoided (I even wrote about that on a post of mine), but undefined is ok, and both optional chaining and nullish coalescing are great tools to deal with possible missing values. Writing your own "abstraction" of nullish values serves no purpose other than over-engineering and over-complicating a simple language feature.

One thing that we need to keep in mind as developers, is that stuff that looks like a "good practice" in a language, doesn't necessarily translate to a "good practice" in another language (and it could even be consider bad). Case in point is that Java is an OOP language with strong use of classes, which I already discussed several times here in DEV are terrible in JavaScript (and I would even say are terrible in general), because they provide a really poor form of encapsulation and reuse, compared with just using functions, modules and composition. So maybe from a class mindset it makes sense to avoid language features such as optional chaining, but it definitely doesn't make sense from a JavaScript point of view, when it actually helps you write less boilerplate to achieve the same goal.

Is almost like when those devs that need to put everything inside classes to the point where if they need to do an addition, they...

// do something like this:
class Operations {
    static add(a, b) {
        return a + b;
    }
}

// Instead of just:
const add = (a, b) => a + b;
Enter fullscreen mode Exit fullscreen mode

But long story short, I would recommend to change the wording of this kind of articles a little to make them look more like "opinion/suggestions", and less like "facts/rules", so we avoid confusing juniors that might think this are like "universal rules" when they aren't.

Cheers!

Collapse
 
mcsee profile image
Maxi Contieri

1 I have no strong evidence it takes more memory.
Memory is expendable. Readability and maintainability aren't
It seems we are dealing with 'Premature Optimization'.

I also think 'undefined' are yet another code smell.
I will also very happy if juniors stop using nulls since we are no longer in the 50s.
But I wil DO take your advice and add a disclaimer these articles are opinions and write it down

I am also reading your articles!

Collapse
 
mrcaidev profile image
Yuwang Cai

I'm a beginner and am using null and undefined in my current project, and I'd love to hear why they are bad. 🤔
For example we have an Order object, and it has a property called finishedAt with type Date. What should I return if the order is not finished? I'm using null in this case.

Thread Thread
 
mcsee profile image
Maxi Contieri

if the order is not finished you shouldnt ask for the date. it makes no sense in real world

An exception should follow fail fast principle

Thread Thread
 
yoursunny profile image
Junxiao Shi • Edited

An unfinished order is signified with .finishedAt = undefined. If you make this property throw an exception, you are violating:
dev.to/mcsee/code-smell-73-excepti...

Thread Thread
 
loucyx profile image
Lou Cyx • Edited

Hi @mrcaidev! You can just omit it. From TypeScript, that Order type would look something like this:

type Order = {
  id: number;
  finishedAt?: Date;
}
Enter fullscreen mode Exit fullscreen mode

So we say finishedAt is optional (is not always there), that means it can be either Date or undefined. Then when you get that from the front-end, you can check if finishedAt is present, instead of checking if is null.

The best thing is that the response will also be smaller:

// You go from this:
{
  "id": 1,
  "finishedAt": null
}

// To this:
{
  "id": 1
}
Enter fullscreen mode Exit fullscreen mode

The property is only sent if it is set, if not, then its omitted :D

Cheers!

Thread Thread
 
mcsee profile image
Maxi Contieri

no. an unfinished order has an status unfinished or it is directly unfinished.

Asking for its data is an Exceptional case. Therefore is not an exception for Expected Case

What is your date of death?
How would you answer it?
undefined? null ? 1970-01-01 ?

No. you should say: 'You cannot ask me this. I am still alive!!'

Thread Thread
 
loucyx profile image
Lou Cyx

Date of death is an optional property that isn't set unless the status is "dead". You don't throw exceptions for optional properties x.x

Thread Thread
 
mcsee profile image
Maxi Contieri

Ok. That's your opinion.
I am not going to change it

Maybe write another smell about "optional" properties

Thread Thread
 
loucyx profile image
Lou Cyx

How do you represent optional data, then? Do you create new classes for every possible scenario? Like User and DeadUser instead of just having a status property and some optional properties that you read depending on that status?

My "opinion" is based on the fact that throwing when accessing an optional property produces a bad DX because now I need to try/catch everything, and also produces a bad UX if we forget about it.

Is really easy to type optional arguments, and IDEs will pick on that and give you feedback if you're trying to read a property that might not be there unless you check or use optional chaining. We can't say the same thing about code that throws. We don't know when something might throw, do we need to do smellier things like wrapping everything in try/catch, using naming conventions to know when someone might throw, etc.

Nowadays folks tend to avoid libraries that throw just because they are unnecessarily problematic, so why would we write code that throws ourselves?

Thread Thread
 
mcsee profile image
Maxi Contieri

It is not User responsibilty to know if its dead or Alive.
Not everything is an attribute

Optional Arguments are 'easy' and 'buggy'. so you type hint but you skip type hinting.

I would not use a library that throws an exception.. unless I am making a mistake

Thread Thread
 
loucyx profile image
Lou Cyx

Oh yeah, we are definitely in different boats here. Having a class for the Order status and another class for the Order is definitely what I was talking about when I mentioned over-engineering. Might make sense if you're working with the limitations that classes have, but doesn't make any sense to do that in JavaScript. This is what I was talking about when I said that "bad practices" in one language don't translate to "bad practices" in another. From my PoV, having a class for the status of another class is bonkers (heck, having an external object for the state of another object doesn't make much sense).

Thread Thread
 
mrcaidev profile image
Yuwang Cai

Thank you for your advice! I was too particular about keeping a fixed structure of the response data, thinking it would benefit in the long run. Now I know what I should do!

Collapse
 
bwca profile image
Volodymyr Yepishev

Well, what about number | null cases? Let's say you're checking pizza price in an app and for an unknown reason, backend cannot find a value for the price and sends null, how would we eliminate null in this case? What would backend then have to send to the frontend if not a null? 0? :)

Collapse
 
loucyx profile image
Lou Cyx

Some folks asked something similar in my article (they even use a pizza example, which I find really funny). My answer is simple:

  • If the price isn't available for an "unknown reason", that's when the response from the back-end should be 500 with an error, not a 200 with nulls to fill the blanks.
  • If the price isn't available because it's an optional field, then you can just not return anything from the back-end:
// When the price is available:
{
  "pizza": "hawaiian",
  "price": 9.99
}

// When the price isn't available:
{
  "pizza": "hawaiian"
}

// Which is way better than sending unnecessary data:
{
  "pizza": "hawaiian",
  "price": null // ????
}
Enter fullscreen mode Exit fullscreen mode

From the TypeScript perspective, the type would look something like this, to represent the optional value:

type Pizza = {
  pizza: string;
  price?: number;
};
Enter fullscreen mode Exit fullscreen mode

Instead of something like this that's actually quite smelly:

type Pizza = {
  pizza: string;
  price: number | null;
}
Enter fullscreen mode Exit fullscreen mode

Folks generally try to "defend" null as a "intentionally blank value" and compare it with undefined as "unintentional left blank", but the reality is that nullish values are the same, and ideally you should be using just one, and I recommend using undefined because is the one the language uses.

Maxi here in the article says that all nullish values are bad, but I don't agree. undefined is great for stuff that is actually "optional" (known as Maybe in other languages, represented with ? in TypeScript). The alternative is to make it more complicated than it needs to be by adding default values, like noops, empty objects, empty arrays, custom Nullish/Optional objects, and so on.

What we both agree on, and if there is a single thing you take from this comment, is that if you see null in your code, that's a smell and almost certainly something that needs to be refactored.

Cheers!

Collapse
 
bwca profile image
Volodymyr Yepishev

I see your point, but defining something as undefined explicitly when you need optional value does not look too attractive.

Besides if you encounter a field which is undefined, there is no telling if it was intentionally set to undefined or was lost somewhere along the way. That'd make you check both for key and value presence to understand if the value is empty, unlike with null, which would require only one check, or with Maxi's default/empty value approach.

Thread Thread
 
loucyx profile image
Lou Cyx • Edited

The thing is, you don't need to make that distinction between "intentionally missing" or "unintentionally missing". You either have a value, or not. Tell me a scenario in which you think that distinction is useful, and I'll tell you how can that be handled without null (you don't need to take my word for it, there are others that think the same). The only check you make is with undefined, which is true for both this scenarios:

const obj = { foo: undefined };

obj?.foo === undefined; // "Intentionally" missing/removed
obj?.bar === undefined; // "Unintentionally" missing.
Enter fullscreen mode Exit fullscreen mode

Both foo and bar have to be deal with the same way, and can be checked with the same logic, so no need for null there either ^_^

Thread Thread
 
bwca profile image
Volodymyr Yepishev • Edited

Lemme illustrate why the undefined doesn't look as a good alternative to null for me so far:

function foo() {}

function bar(){ return undefined; }

const x: undefined = foo();
const y: undefined = bar();
Enter fullscreen mode Exit fullscreen mode

in typescript returned type of foo will be inferred as void (though the absence of value is undefined), which means assigning void to undefined is a type error.

What's worse is you can go ahead and write:

const y2: void = bar();
Enter fullscreen mode Exit fullscreen mode

So undefined is assignable to void and undefined, but void is not assignable to undefined. But they are both undefined, that's even more confusing than having null imo :)

Thread Thread
 
loucyx profile image
Lou Cyx

There's a few things to address here:

  • void and undefined aren't the same thing. void is a type to signal developers that you shouldn't be using the return value of a function. undefined is a value that you can use when the expected value is "missing" (for example, a find function can return undefined when no item was found). So assigning the return value of foo in this case to a constant is a mistake and is great to receive errors for that.
  • If your intention is to be able to assign the return value of foo to something, then you have to be explicit about it:
function foo() {
    return undefined;
}
Enter fullscreen mode Exit fullscreen mode

This is actually good, because you avoid returning undefined implicitly, you only return undefined as a value if that was what you wanted.

  • In a more real world scenario, if you want to assign the return value of foo to something, that function would look something like:
function foo(condition) {
    return condition ? "something" : undefined;
}

// or

function foo(condition) {
    if (condition) {
        return "something";
    }
}
Enter fullscreen mode Exit fullscreen mode

Both of the above return string | undefined, not string | void.

  • Try writing those same snippets using null, you end up writing almost the same code, but with the difference that null has a lot of issues that undefined doesn't.
  • When I said "Tell me a scenario in which you think that distinction is useful", this isn't it. In here you're showing how a function that returns undefined implicitly in JavaScript, in TypeScript is inferred as void, but that has nothing to do with differentiating between "intentionally" and "unintentionally" missing values.
Thread Thread
 
bwca profile image
Volodymyr Yepishev

I'll give your approach a try and maybe come back with some arguments to your article :)

Thread Thread
 
mcsee profile image
Maxi Contieri

Hi

I am not saying undefined is better than null.
They are bad and introduce lots of IFs.

Thread Thread
 
mcsee profile image
Maxi Contieri

undefined is not a good alternative to null

We should avoid both

Collapse
 
mcsee profile image
Maxi Contieri

You mean number | number cases?
Nulls don't exist on real world

if backend cannot find a value it should an exception. Which is exactly what happen in real world

Backend should inform with exceptions. Nulls are schizofrenics. Exceptions are specific and handleable

Collapse
 
ecyrbe profile image
ecyrbe

I completly disagree here. Not about null being bad, but about exceptions as a replacement for null.

Null is not always conveying an error, you should take a step back when writing theses.

Null and undefined is often used for indicating optional values. Optional is not an error. And that is why a lot of language and libraries are supporting two kind of result types :

  • Result types that can convey an explicit error (better than exceptions in my opinion)
  • Optional types that convey missing information when missing is not an error

And you can declare both like this in typescript :

type Ok<T> = { kind: 'OK', value: T };
type Err<E> = { kind: 'ERROR', value: E};

type Result< T, E > = Ok<T> | Err<E>;
Enter fullscreen mode Exit fullscreen mode

and

type None =  { kind: 'NONE' };
type Optional<T> = Ok<T> | None
Enter fullscreen mode Exit fullscreen mode

But those are not javascript native features, and then can have an impact on performance. If performance matters, using undefined and checking for it should be acceptable.

My own veteran opinion.

Thread Thread
 
mcsee profile image
Maxi Contieri • Edited

Hi Veteran

I am also a veteran and saw too many systems failing for the infamous null

The man who invented it regretted in public

Here is the whole story:

And IMHO and experience. Performance (and premature optimization) are much less important than reliability and readability.
Performance was important on the 50s where nobody needed to maintain code.

Thread Thread
 
ecyrbe profile image
ecyrbe • Edited

I perfectly know about this. But you should also know that in javascript, null or undefined to not lead to the same kind of errors. Since this can't lead to undefined behaviour and arbitrary code execution that C has.

Null or undefined in javascript lead to unchecked results and application Panic. Panics are there to prevent exploits. So it will not lead to a CVE and an exploit unless you have a functionnal happy path that leads to an exploit. Which is not the same.

To prevent the later, I recommend everyone to use Typescript and strictNullChecks by default to allow your static code analysis to detect these kind of defects.

In my opinion it's better than using runtime exceptions everywhere. Exceptions are hard to follow, they can't be statically analysed, they can lead to the same kind of issues if not checked, aka a system Panic.

Thread Thread
 
mcsee profile image
Maxi Contieri

"To prevent the later, I recommend everyone to use Typescript and strictNullChecks by default to allow your static code analysis to detect these kind of defects" awesome

Thread Thread
 
ecyrbe profile image
ecyrbe • Edited

And to clarify my point. That i think your article about Null : The billion dollar mistake misses to highlight.

What was bad with NULL is design it in such a way that it can lead to undefined behaviour and arbitrary code execution. What was also bad was not adding a basic static analysis tools to detect when NULL is unchecked.

And to expand on what kind of issue unchecked NULL can lead to in javascript :

  • Unchecked NULL results : lead to functionnal errors,
  • Unchecked accesses of NULL values or functions : leads to system Panic

The main issue is with the first one. Since the second one will lead to a Fail Fast without any exploit possible.

So the first one is tricky, and is not just tied to NULL. Indeed, functionnal error (not handling all uses cases) can also be common on every langages, even those without NULL.

The solution to theses types of errors are nowadays to use a strict static analysis tools. And i repeat, these errors are not just tied to NULL.

Every modern langages now come with a static code analyzer embeded, I have given typescript example, but since i use a lot Rust langage, i can also say that it comes with one of the best static analysis tool out there (borrow checker, memory lifetime checking, etc are so wonderfull tools for static code analysis).

Collapse
 
pengeszikra profile image
Peter Vivo

Imho work with user as class seems a little bit wierd. Behind of optional chaning usability is the outer data structure deep checking with sort code, I did not means as hack.

if (typeof user.functionDefinedOrNot === 'function') {
  user.functionDefinedOrNot();
}
Enter fullscreen mode Exit fullscreen mode