DEV Community

loading...

Question: Is this a terrible idea?

harleybl
・1 min read

I have a bunch of utility functions in typescript for doing various things. I find that I often want to check if an array has any elements in it, but I am not sure if that array is undefined or null. I wind up with blocks similar to this in my code:

if (!someArray || someArray.length == 0) {
   doNothing();
} else {
   doSomething();
}

My first instinct was to write this method.

export function any<T>(arr: T[]): boolean {
    if (!arr) return false;
    return arr.length > 0;
}

The original code block becomes this, which I find more readable:

if (any(someArray)) {
   doSomething();
} else {
   doNothing();
}

I chose 'any' because I spend a lot of time in c# land and it roughly matches the LINQ method enumerable.Any(). However, using a reserved word for a function name seemed like a bad idea. Is it a bad idea?
Even if the transpiler allows it, maybe it is even less readable since any is the "I don't know what type this is" type.

In this instance, I chose to rename the function to atLeastOne which is a little more descriptive and doesn't clash with a reserved word, but I am still wondering if it was a bad idea.

Discussion (5)

Collapse
sargalias profile image
Spyros Argalias • Edited

I don't see a big problem with it. If you and your team are happy, then by all means use anything :).

But since you're asking, I would personally name the function something like safeHasElements(array). The reasoning for this is because you (or someone new to the code) might expect atLeastOne to take an array only, and error if it gets null or undefined. But in functional programming, the word "safe" normally implies that things won't throw errors but fail silently instead, or in your case just return false / undefined.

Collapse
harleybl profile image
harleybl Author

Thanks, Spyros - I like your naming suggestion.

Collapse
pigozzifr profile image
Francesco Pigozzi

Why don't you reverse the logic and name it isEmpty instead?

Collapse
harleybl profile image
harleybl Author • Edited

I have another method for the isEmpty case, but often I find it more readable to have if statement logic checks for a positive condition rather than a negative one. I read somewhere, I think it was Clean Code, that using positive logic requires less cognitive load when reading through the code. Especially when you are checking for more than one condition.

Which is more readable?

if (!isEmpty(arr) || !isEmpty(arr2) {
  doSomething()
} else {
  doNothing()
}

or

if (any(arr) || any(arr)) {
  doSomething()
} else {
  doNothing()
}

I prefer the second.

Note: isEmpty is an rxjs operator since that could be confusing I will call it something else like safeIsEmpty.

Collapse
pigozzifr profile image
Francesco Pigozzi

Just like you said: positive logic requires less cognitive load

For that reason, checking for a negative logic first and quit if true should be safer, more concise and readable than nesting two different logic inside an if/else statement

Forem Open with the Forem app