Conciseness and readability

I've written in the past about how to refactor some code to improve readability using array functions, but I also wanted to add some clarity to my thoughts.

We have a lot of tools at our disposal, sometimes we misuse our tools, it happens. Let's talk about reduce and readability. It's such an (ab)used function that it merits a larger discussion.

// An example of a reduce function
[2,3,4].reduce((sum, value) => sum + value, 0); // 10

This is the most common use of reduce. It's simple to understand, right? We add all numbers in the array and start with the initial value 0;

Let's dig a little deeper.

First off, the initial value - defaulting to first item in the array, unless specified - is placed at the end of the parameter list. Its location is understandable since its existence is optional and if it was the first parameter, most uses would look something like reduce(undefined, (sum, value) => sum + value);.

This position means, that there is that annoying bug in which you forget to set a proper initial value causing an undesired outcome or, even worse, a runtime error.

Unfortunately, reduce is often abused as well. Let's look at such an example:

const MAX_HOURS = 24;
const TIME_STRING_LENGTH = 5;

export const timeOptions = Array.from({ length: MAX_HOURS })
    .reduce(
        (acc, value, index) => {
            const hour = `${index}:00`.padStart(TIME_STRING_LENGTH, '0');
            const hourAndAHalf = `${index}:30`.padStart(TIME_STRING_LENGTH, '0');

            return {
                ...acc,
                [hour]: hour,
                [hourAndAHalf]: hourAndAHalf
            };
        },
        { '': 'Off' }
    );

How long did it take you to understand this code? It should be a few seconds, but I wager it took a bit longer than expected.

The code above, creates a key/value pair of times. Ex: 00:00, 00:20, 01:00, 01:30, etc.. But it begins with "Off".

Multiple things are wrong here. Let's look at the reduce usage first.

Why would someone chose to write it like this? Here are some theories:

1. Looks smart.
2. Array functions abstract complexity away so they are preferred in favor of terseness.
3. Lint rules disallow while and for.

Lint rules

The worst offender is lint rules and staying within those rules even if the outcome is not very readable.

If the purpose is to maintain consistency, then do whatever you feel is ok for you and your team. But leave escape hatches for instances where the simpler syntax works. This could be the subject of a larger article, but I think that disabling lint rules in such instances is beneficial to the overall readability and maintainability of a given codebase.

I'd much rather have clearer code with a eslint-disable rule rather than the code above.

Looks smart

I mean look at that code! It's obvious I know what I'm doing, right?

I use Array.from and I pass it an object with a property of length.It's pretty obvious I know about spread and dynamic keys. I'm using the platform to its maximum potential, right? Isn't this ok? The tools are there, we should use them!

It just looks smart

It's a pretty obtuse, but interesting way to achieve the same thing as a bog standard for loop. The complexity one needs to manage to read and write this code is far larger than it is in the case of a for loop. We get it, it looks nice, but it's not very readable. Or, more importantly, it's not easily readable by all.

Array Functions are our friends!

This is similar to looking smart, but it stems from the fact that we see tools, and we instantly want to use them everywhere.

To a man with a hammer, everything looks like a nail.

Yes, most array functions are our friends since they are more expressive and abstract away some of the complexity. This is correct, but the issue at hand is we aren't using the function to its fullest. How come?

We don't use the value argument at all. This is the single largest hint that the code is not exactly a good place to use reduce. If we don't use the value, this is just a loop.

Plain English please

Don't discard for and for .. of as being from another time period. Just because you heard that reduce can do so much.

It's just a loop. Let that sink in. It's just a loop.

Let's write it as a loop!

const timeOptions = {
    '': 'Off'
};

for (let index = 0; index < 24; index++) {
    const hour = `${index}:00`.padStart(TIME_STRING_LENGTH, '0');
    const hourAndAHalf = `${index}:30`.padStart(TIME_STRING_LENGTH, '0');

    timeOptions[hour] = hour;
    timeOptions[hourAndAHalf] = hourAndAHalf;
}

Looks a bit simpler, does the same thing, and is a bit easier to read for anyone, regardless of level.

Writing good code should always include a concern for readability. You'll thank yourself later!