DEV Community

Discussion on: Keeping Your Code Simple

Collapse
 
moopet profile image
Ben Sinclair

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.

Collapse
 
tiffany profile image
tiff

Yes. My friend told me I should break them out into separate variables so I tried going with that. He gave me a solid example. The difference between what I do and what he does are magnitudes different though: he's responsible for the lives of train passengers and I am just a web dev. I can understand him wanting to simplify it into separate variables as his code needs to keep people physically safe and a bad bug could cost lives. But this reads a bit better.

Collapse
 
inakiarroyo profile image
Iñaki Arroyo • Edited

Not trying to be rude here, just giving my point of view, but, I am so far to be agreed with you here, as a web dev or just as a dev you should always consider writing your code in a way that it doesn't break "the train", hiding the possibility of making bugs due to you are just a web dev is a very poor excuse. From my point of view, thinking in that way ends in having bugs over bugs into the projects.

Collapse
 
willvincent profile image
Will Vincent • Edited

Even if you're "just a web dev" your code really ought to be given the same amount of care as if it were responsible for people's lives. :)

But if you really want to be clever, and keep it readable, it can be dramatically shortened with lodash's maxBy method...

const _ = require('lodash')

const strings = [
  '12345',
  '123',
  '1234567',
  '12',
  '1',
  '123456',
  '1234',
]

const result = _.maxBy(strings, string => string.length)

I'm sure the code challenge requires no external libraries, but that's not necessarily anything like a real-world requirement.

Interestingly, lodash uses a for loop for this, for what it's worth..