Compact or Verbose Codestyle?

Which code style do you prefer and why?

Compact

const get = (store, key) => 
    Reflect.has(window[store], key) && 
        JSON.parse(window[store][key]) ||
        new Error(`Key "${key}" not found in window.${store}.`);

Verbose

const get = (store, key) => {
    if (!Reflect.has(window[store], key)) {
        return new Error(`Key "${key}" not found in window.${store}.`);
    }

    const jsonStr = window[store][key]
    const dataObj = JSON.parse(jsonStr);
    return dataObj;
};

Comments

[Muntadhar Haydar]

depending on my knowledge of the language (and the framework) but will probably go for the least amount of code that's readable

[Javi]

As long as it's readable, make it the most compact possible.

Verbose can be unreadable too.

[Réda Housni Alaoui]

The second with typescript

[Harry Park]

I don't believe it length of the code matters as much now as long as the algorithm is correct and the code is readable. There are plenty of optimizers and minifiers that a project can use to reduce production code.

[Jesse M. Holmes]

Isn’t it strange how the former seems practical if you’ve spent all day working in React using conditional rendering?

If I walked into that piece of code, it would churn my brain for a while, and yet the same expressed as a ternary operation would make sense to me immediately.

If I were a new developer, only the latter would give me hope.

I think compact code is beautiful, but I prefer verbose code because it is quickly understood. There is a balance, and if I feel like compact is the way to go, I’ll balance with comments. In the case of a ternary operator, I explain it once per file so that a more junior developer will have a clue to cling to.

[Gal Schlezinger]

I guess you would have want to use the ternary operator to make it more readable, instead of adding the || and && which are harder to understand: the ternary operator accepts a nicer and more predictable form of (value, ifTrue, ifFalse) instead of constructing it on your own with boolean operator magic

const get = (store, key) =>
  Reflect.has(window[store], key)
    ? JSON.parse(window[store][key])
    : new Error(`Key ${key} not found`)

I guess this is readable enough, but I'll probably use the "verbose" one (it really depends on the project standards): I like early returns, or "guard" clauses - can help debugging by understanding that you can stop evaluating the function in your head 😺

[Templar++]

Always program as if the next person, who will be supporting your software is a psychopath, who knows where you live.

When your code isn't easy to understand and obvious from first sight - it's cr*p

[phantomread]

Verbose, although you can combine the second and third last lines into one.

[Alain Van Hout]

Between those two choices, I'd definitely go for the second one: the time you save writing less characters will not make up the hours you might lose in bug hunting.

As others have already pointed out though, there are two approaches used here:

• The guard clause (the if statement) helps separate the concerns within the method, since validation and data retrieval are indeed separate concerns.
• Extracting intermediate variables can be really helpful when the variable names communicate their (domain) purpose. In the example, that's however not really the case.

I think the important thing to keep in mind is that while the first example is much shorter (in amount of characters), it has exactly the same logic and the second example. That's to say: it's much more dense, meaning that you have to mentally de-compress it when you want to make changes to it. That doesn't help maintainability, and might even slow you (or your colleagues) down in the long run.