I’d just call it bad code and start over…

The "improvement" doesn't really help or support your point. More verbose variable names, removing the slew of variables for nothing, ditching the "const" rubbish that does NOTHING good in this case, (not a fan of let/const… to me they’re actually pretty stupid wasting overhead for little to no legitimate benefit), could actually be less code AND far clearer/easy to understand.

The big flaw I see here though is that you're checking name and telephone for their state up to five times instead of just twice. The old rule, “don’t check the same condition more than once if it can be avoided”

.. and you hardcode all the cases making it harder to expand.

If you checked them and then dumped the name into an array with a meaningful name — such as “invalidFormFields” — you could use a loose compare to test for an error state, and join to put them together in the output.

Less code, clearer as to what’s going on, easier to add more conditions, leverages the “return of append” to avoid wasting memory on variables for nothing.

Though in practice I’d fold out that error output into its own function… make it return the fetch or false so you can “then”, etc, etc.

Even cleaner/clearer inside the functionality, adds separation of concerns, proper state feedback to the callee, and it’s STILL less code.

I know you said the starting point was intentionally bad, but that is part and parcel of what card stacked your claim into a corner. There are legitimate reasons for excess variables that might seem bad but actually help both clarity and performance, this wasn’t it.

Places where ‘variables for nothing’ can pay dividends:

  1. Array lookups are slow in JavaScript because they’re not ACTUALLY arrays, they’re pointer based list. Storing a copy of the value if you use the lookup more than once can add clarity (via the function name) and speed (by not wasting time on extra lookups)
  2. If the same string is re-used, storing it can be less code. You actually did this with “basicString”, but WHY basicString existed was flawed.
  3. variables outside the scope might have different purposes/meanings and could be slower. This is particularly true of system objects like document. Copying them to a local variable can speed up their lookups if used a lot (locals are checked before globals). See the trick of passing (document, window) to an anonymous function as (d, w)
  4. When the same comparison or result is needed multiple times. I think you tried to do this, but again the reasoning/logic of the code itself hobbled the examples.

But to be fair, I started out in RCA 1802 machine language, learned Wirth family languages before anyone outside of “big iron” gave a flying purple fish about C, and spent a good chunk of the ‘90’s working in Ada, so I have radically different viewpoints on what efficient code is. Much less what code clarity is… at least compared to the rank and file who came up on the C syntax side of things.

I don’t think I’ll ever grasp how/why C syntax became the norm given how painfully and agonizingly cryptic it is. Much less how ECMAScript is only making it worse with garbage like arrow functions, nullish coalescing, optional chaining, and all the other cryptic symbol based trash that leaves me saying “couldn’t we just use words for this stuff?”

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store