I've been a professional C, Perl, PHP and Python developer.
I'm an ex-sysadmin from the late 20th century.
These days I do more Javascript and CSS and whatnot, and promote UX and accessibility.
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:
letlenX=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.
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.
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.
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.
I'll tell you straight off why I think the first one is tricky to read:
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:makes me think that
lenX
is going to be modified later, because otherwise it would beconst
. The definition outside thereduce
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 ofstr1
(whatever that was). Unless I don't get howreduce
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:
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.
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.
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.
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...
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..