Keeping Your Code Simple

This is a really great example of this concept. Example two is so much cleaner, and really passes the squint test.

Small functions are good but cramming too many things on one line is definitely not.

Anonymous lambdas can be okay, but I often create a named function to make things a bit more clear. For example, here you could write something like:

const strs = ['hello', 'goodbye', 'farewell', 'aloha']

const longerOfPair = (str1, str2) => str1.length >= str2.length ? str1  : str2

const longest = strs.reduce(longerOfPair, '') // > farewell

I remember when you sent that tweet out!

it also makes the code unreadable and you start to look less and less appealing as a potential dev candidate for X company.

facts! Thanks for the advice!

This made me think of this tweet:

getify

@getify

How to answer a trick/gotcha JS question in a job interview:

JS is often thought of us as being quirky and tricky, and thus has a bad reputation. Hence your question. However, it can be written to cleanly/clearly communicate ideas without such antics. I would re-write this code.

22:47 PM - 14 Aug 2018

35161

getify

@getify

Instead of reinforcing bad practices, I'd love to discuss how I use my knowledge of JS to communicate effectively with future readers of my code.

------

If I heard this from a candidate, I'd hire on the spot.

22:47 PM - 14 Aug 2018

1493

In your example let lenX = str1.length; is confusing to me because it is declared outside of the reduce where str1 is not accessible.

was your intent to write it like this?

const longest = strs.reduce((str1, str2) => {
  const lenX = str1.length;
  const lenY = str2.length;

  return lenX >= lenY ? str1 : str2
}, '');

I also like how Nested Software broke out the reducer into it's own function.

I'll tell you straight off why I think the first one is tricky to read:

const longest = strs.reduce((x, y) => x.length >= y.length ? x : y, '' );

We've got => followed by >=. My brain gets easily confused and I flick between bits of the line thinking which is the comparison and which is the fat arrow.

I think that the x.length parts are quite straightforward and don't think that breaking them out into their own variables aids readability. The line here:

let lenX = str1.length;

makes me think that lenX is going to be modified later, because otherwise it would be const. The definition outside the reduce also means that since you've decoupled the lengths from the strings, you can't reduce more than two array elements. By the time the reduce gets to the third element in your array, well, lenX is still the length of str1 (whatever that was). Unless I don't get how reduce is working here (which is entirely possible...)

There's another tiny problem I spotted too: your comment says "is x string length greater than y string length?" but your code tests >= so if both strings are the same length, your function would return one but your documentation says it should return the other one.

I think that I'd probably go with:

const longest = strs.reduce((str1, str2) => str1.length > str2.length ? str1 : str2, '');

and not declare any other variables. I might split it over a couple of lines too, because ternaries inside fat-arrow functions make for a busy line and make my brain work more than I like.

I would probably end up using a for loop for this, but here's the most readable version using reduce I could think of:

function longestString(strs) {
  return strs.reduce((x, y) => {
    if (x.length > y.length)
      return x;
    else
      return y;
  }, '');
}

All good stuff. When you write code it is always best to remember that the person who will be looking at it next will probably be you...

A you who has probably forgotten how this app works, has a system down, users screaming and a boss looking over your shoulder.

Be kind to your future self, keep it as simple as possible because future you will really appreciate it.

This code does not work. This line throws an error:

let lenX = str1.length;

Even in a case of the 2 longest strings with the same length the results are partially correct. In that case, you need to return a list of strings.

A long time ago I read a post here on dev.to about the danger of writing clever code. The main idea of the article was this:

If you write clever code, the bugs that will come out of it will be clever.

I always try to write code that is "clean" but I know that is impossible. My perception of what is simple and clean will never be the same as other people (or even my future self).

What I do is avoid clever solutions. Most of the time the "cleverness" in the code is unnecesary complexity.

Bravo :D

Maybe I'm missing something, but doesn't passing an initialValue after your reduce's callback mean that what you're calling str1 (in the callback) is actually the accumulator? For the sake of verbosity, I'd name it accordingly. Better yet, I'd get rid of the initialValue altogether.

Please correct me if I'm wrong.

I agree that the code you mentioned is terse, but in my humble opinion it is readable.

Everyone has their own definitions of readability. I think it is totally okay to have a terse lambda like the one you mentioned, as long as it is simply doing one thing and is reasonably small. In the example you posted it is simply comparing lengths of two strings and returning the longest. These type of short lambdas are pretty convenient and are pretty common in Javascript these days.
If you compare them side by side, the lambda version wins for me.

function longest(strs) {
  return strs.reduce((x, y) => {
    if (x.length > y.length)
      return x;
    else
      return y;
  }, '');
}

const longest = strs => strs.reduce((s, t) => s.length >= t.length ? s : t); 

A code is clever when you yourself don't understand what it does a week later. To mitigate that you have to provide a verbose comment describing what the hell it does and pray to god that whoever changes the code also updates the comment, because in real life comments and code go out of sync pretty often.

Another important thing about the more terse option is that it improves readability when you start chaining operators. Let me give you a hypothetical example and you decide for yourself:

function longest(strs) {
  return strs
    .filter(s => {
      if (typeof s === 'string') {
        return true;
      }
    })
    .map(s => {
      if (/^[aeiou]/i.test(s)) return 'an ' + s;
      else return 'a ' + s;
    })
    .reduce((x, y) => {
      if (x.length > y.length) return x;
      else return y;
    }, '');
}

const longest = strs =>
  strs
    .filter(s => typeof s === 'string')
    .map(s => (/^[aeiou]/i.test(s) ? 'an ' : 'a ') + s)
    .reduce((s, t) => (s.length >= t.length ? s : t));

• Each individual piece is easy to reason with independently.
• It has more inertia to code change, i.e. a fellow programmer cannot simply introduce a closure variable as easily as in the if else version. Hey we all have had that bug which was temporarily fixed by peppering code with global variables.
• Less syntax noise.
• I don't have to constantly fiddle around with closing {}.

I don't think your first function is hard to read at all. If anybody finds it unreadable enough to say

that one solution was a mess and that if he had written anything like that he would have gotten chewed out by his boss.

I would chuckle and dismiss their opinion as them probably not being used to working with reduce.

That being said, if I were to improve that code I would probably extract a function that would compare two strings and return the longest, like Nested Software suggests. lenX doesn't really provide more information than x.length.

Regarding short variable names, there is a case for them in certain contexts, and reduce and the like are one of those. Let's consider this.

const pony_names = ponies.map(x => x.name)

Could I write in a way that avoids naming a variable x? Sure.

const pony_names = ponies.map(pony => pony.name)

However, does it add anything? Since I'm mapping over an array named ponies, I know that x is going to refer to a pony, and it works much like a pronoun in a natural language. In natural languages, when the immediate context makes the subject of a sentence clear, it is commonly replaced by a pronoun (or in some languages skipped altogether). So instead of I found a book. The book has a blue cover and the book has 307 pages, we usually say I found a book. It has a blue cover and 307 pages. Naming a variable x would be much like using it in English.

As I've said, to be able to do that effectively, what x refers to needs to be obvious by the immediate context. I still need to give pony_names a sensible name because I'm going to use it in another context detached from the ponies array and then I wouldn't know what x refers to. But for short functions where the immediate context makes it clear what the variable is, longer names are not really a must.

What about reduce then?

ponies.reduce((x, y) => ({ ...x, [y.name]: y }), {})

I agree that x and y might be somewhat confusing here. We have one topic (ponies), but two pronouns (x, y), what do they refer to? We know that reduce passes the element it is iterating over in the second parameter, and the result of the last invocation in the first parameter, so let's give that parameter a more meaningful name.

ponies.reduce((prev, x) => ({ ...prev, [x.name]: x }), {})
// or
ponies.reduce((acc, x) => ({ ...acc, [x.name]: x }), {})

No more having to check the docs again to see which parameter was the accumulated result and which one was the current element of the array.

Let's consider another case, the function that takes two strings and returns the longest.

const longestString = (a, b) => a.length >= b.length ? a : b

Could I have named my variables string_1 and string_2? Sure, but I would argue that those are not meaningful names either, they are just longer. For all I know, a and b could be pony names or science fiction novels. And it's fine, because longestString is a generic function that should work for any kind of string. Do I need to give the variables a name that indicates they are strings? Again, the function name is just a few characters to the left and it already talks about strings, so the immediate context is enough to understand that a and b are strings.

TL;DR

To summarise my point, I think one-character function parameter names are alright when two conditions are fulfilled.

• The name of the function, or the context where an anonymous functions is invoked, makes the nature of the parameter obvious.
• The body of the function is short.

And your first code fulfils both conditions. If anything, I would replace your reducer function's signature from (x, y) to (prev, x) or extract it into a named function that takes two strings and returns the longest.

"While yes, it makes you look as if you know what you're doing, it also makes the code unreadable"

I couldn't agree more. Nobody should ever write code just to make themselves look good/clever.
There is no place for ego inside a company.
Never forget that you're not writing code (just) for yourself. You're (also) writing for others. And when writing code for others, you should make it as readable and as easy to understand as possible.