Rida F'kih

Posted on Oct 20, 2022 • Edited on Oct 24, 2022

I Re-Wrote These 10+ Single Lines of JavaScript Code, the Team Lead Praised the Code for Being Elegant

#javascript #tutorial #beginners #programming

I am the Team Lead, I praised the code. /s

This post is in response to, but by no means a dig on the blog post written by Zard-x on Medium called ‘I Wrote These 10+ Single Lines of JavaScript Code; the Team Lead Praised the Code for Being Elegant’.

One-liners are really cool challenges and very fun to make. I used to spend a lot of my time on codewars.com and codingame.com, which are two websites that facilitates little coding challenges which challenge your ability to write short code, or write code fast.

While code golf can serve as a fun engineering exercise, their lessons should rarely be used in the real world—and when they are—by no means should engineering managers be promoting developers to use this type of code in a codebase. When you’re in a collaborative environment, writing good code should be a priority. That means code that is readable, maintainable, and runs reliably.

Code golf is a type of recreational computer programming competition in which participants strive to achieve the shortest possible source code that solves a certain problem.

— Wikipedia, Code golf

Array Deduplication

Original Source Code

I’m not necessarily opposed to the common trick of removing duplicates from arrays by instantiating, and spreading a set, but due to the amount of times you’re indexing the array, it can be quite poor performance-wise.

Rewritten Source Code

On an array of 1,000,000 randomly generated numbers between 1 - 100, the original source code only performs at 12% of the speed that the rewritten version does. Since we use a map in the second function, we only have to iterate once over the array, and made it so that checking to see if we’ve already seen an element takes O(1) time.

The code eliminates “tricks,” and instead implores a well-known concept like hash maps, which may reduce the occurrence of inquiries by other developers on your code. The code also has well-named variables, with the function name removeDuplicateStrings defining the action that the function is doing, rather than an ambiguous name like uniqueArr, it also does not use excessively abbreviated words, making the code easier for non-native English speakers to understand.

Notice that the function is not documented. Excessive documentation can sometimes be a bad thing. If your variable names are well named, you may find your comments redundant, and that’s a good thing! If that happens, happily drop ‘em, and ship it!

If we’re absolutely dead-set on using spread operators and sets, or we want to remove duplicates of values that are not exclusively strings without writing bloated functionality, all we need to do is document our code, and ensure our variables are well-named.

Get Parameters from a URL and Convert Them to Object

Original Source Code

If there was a bug with this code, how excited would you be when you figured out this is the function you’d be working on? Odds are, not so much. Not to mention that re-building JSON objects with strings is something you want to avoid as much as possible, in this case that’d be quite easy.

Rewritten Source Code

In the rewritten code for this function, we use the convenient native JavaScript URL API which automatically invokes an instance of the URLSearchParams object for us, then we simply use the Object#fromEntries function on the resulting object! This prevents us from having to manually construct a JSON object by mingling and mangling strings.

Notice that we’ve provided a comment for this function! Despite the fact that I’ve included a specification for which type of parameters the function handles in the name of the function getURLParameters, it’s not necessarily clear what format the resulting object would be in.

Check Whether the Object is Empty

Original Source Code

Using ownKeys or Object.keys are both not very performant when checking if an object is empty.

Rewritten Source Code

This rewritten code is an old school method, and man has it passed the test of time! The re-written source code performs 20 times faster than the original method, and that’s only with two properties on an object. The performance boost comes from the fact that you no longer need to create arrays, copy over values, and index keys. You see one key and you’ve escaped the function!

It might not immediately be obvious why we’re iterating over the keys of the object, so we’ve left a short comment in there to ensure that any other developers know what’s going on.

Reverse the String

Original Source Code

In this case, we’re not off to a bad start! We could use a for-loop to get an 80% performance boost on short strings, and we might consider implementing that method if we’re reversing some chunky strings often, but in the case of the odd reversal on lightweight strings, we might just be better off with this functionality.

Rewritten Source Code

Splitting the chained methods across multiple lines is a personal preference of mine, as it allows me to scan the function paths rather than jumping through them, however if someone asked me to change it I wouldn’t make a fuss.

For fun, here is a more performant version.

Generate Random Hex

Original Source Code

Once again, not off to a bad start! I particularly like that we’re directly referencing the maximum hexadecimal value supported for non-alpha colours! Some minor cleanup, and we’ll be in business!

Rewritten Source Code

The only major change I made in this example is that I changed padEnd to padStart. The way this function works is by generating a random 24-bit integer, and then representing it in its hexadecimal format.

The minimum value we can get is 0 0x000000, with the highest value being 16,777,215 0xFFFFFF.

Let’s say our randomValue ended up being 255. Our hexValue would then be ff, and if we padded the end, our return value would be #ff0000, versus if we padded the beginning it would be #0000ff. If we then translate both these values back into integers, #ff0000 would actually represent the value 16,711,680, while #0000ff would represent 255. This means that if we pad the end rather than the beginning, we’d effectively get the wrong value.

In addition to this fact, with the first method, by padding the end rather than the beginning, #0000ff would actually be impossible to generate. Meaning there are plenty of missing values that could not possibly be generated. Specifically, every value between 0x000001 and 0x100000. That is 1,048,576 missing values from our function, but would also tip the odds in favour of all values that end with a 0, some more than others. For example, #100000 would be the result from 0x1, 0x10, 0x100, 0x1000, 0x10000, and of course 0x100000.

Conclusion

That took longer than expected, so I’ll be posting a second part to this blog post at a later date. I had a ton of fun refactoring this code. Would you have done something differently? Leave a comment on Medium or dev.to! I think this is a great learning opportunity for myself and others that might be in their code-golfing phase.

Want to read more of my stuff? You can check out my personal website. If you’d like to start a dialogue, feel free to tweet at me!

Latest comments (52)

Hutch • Dec 2 '22

The new getRandomHexColor function has a slight difference: it doesn't prepend the string with a #.

Interesting that the new string reversal function is faster, since, naively, it would create n strings, where n is the number of characters in the string. The engine must be doing some optimizing in the background, which you wouldn't know without benchmarking (or digging into its implementation).

Jonathan Irvin • Oct 27 '22

I'm a big fan of "human read code, machines interpret."

That being said, your refactors are clean where your comments are, but call me old-fashioned, I prefer JSDocs. That way you have hints where your functions are being used and you can expect the correct output.

As far as performance goes, ¯_(ツ)_/¯ . I stand by readability-first because performance only lasts as long as the person who wrote it remembers :). The moment they leave, it becomes a prime target for refactoring into something that's more common-knowledge.

Rida F'kih • Oct 28 '22

I also prefer JSDocs, but for this article acknowledged I can be the odd-one-out here and gave it a bit more familiarity.

Kacper Turowski • Oct 26 '22 • Edited

Lord! Yes! Thank you!

I really don't understand why some people think we should just cram as much code in single line as it's humanly possible. Is it because people tend to say that files should be around 500 lines long and not more?

Why does someone think that a 2000 character long line is better? All that my ADHD brain sees is

const getParameters = LoremipsumdolorsitametconsecteturadipiscingelitSedvariusmetuspurusvelpulvinarleosodaleseuVestibulumsagittiselitquisleoviverramalesuadaMorbilaoreetposuereloremvitaeposuereSedpellentesquearcunontinciduntgravidaMorbisedorciipsumFuscejustonibhullamcorperattristiquequisaccumsaninaugueMaurismattiseuismoderatquisdapibusNullamvitaenequedolorSedquiseuismodmetusvelluctusmetusSedvolutpatidnislquisefficiturCurabiturlacinianisietrutrumviverraestnuncconvallisrisusaimperdietmagnaestnecmaurisPellentesquequisligulanecurnablanditiaculisInlectusduitristiquesitametduisitametaccumsaneleifendlacus;

Working code is not code golf. Heck, even katas are not code golf.

devtolove • Oct 25 '22

incredibly

nogou21 • Oct 25 '22

i'm having mixed emotions about the this article, i agree with part of it but for the string reverse is a big 'NO' for me . the solution works better only for very short strings , i think the original article made good use of native string methods.

Rida F'kih • Oct 25 '22 • Edited

In my benchmarks, I ran the two functions on two strings. A very short string, and a series of increasingly excessively long strings. You're right, there is diminishing returns of performance and in my benchmarks it bottomed out to a 20% increase in performance rather than the 80% you would see on shorter strings.

Remember, these refactors are not absolute, especially pertaining to the string reversal. The more performant version was, as mentioned in the article, only included for fun, and I do outline there is absolutely nothing wrong with using the native methods.

As developers, I know we get excited about the meat of the code, but as mentioned to another commenter, there were other equally important aspects to the article than performance.

"major takeaways and talking points are the naming of variables, length of lines, scan-ability of code, legibility of code, actual methods, genuine mistakes in the code, and to not prioritize code shortness over performance."

In the context of the string reversal specifically, the--albeit subjective--improvements I made to the original method were making the function name, and variable names more clear, splitting the chained method calls across multiple lines to make the function more skimmable, outlining that the latter point is completely optional and not anything to make a fuss over. I then provided the more performant version for fun, as outlined!

Jason Van Malder • Oct 25 '22 • Edited

I just made a little benchmark: Using Set() vs not using Set() to remove duplicates from an array.

Remove duplicates from array (Set VS loop)

Set: 16 ms
Loop: 4333 ms

Source:

const arr = [...Array(1000000).keys()].map(() => Math.floor(Math.random() * 10000));

// Remove duplicates from array with Set
const removeDuplicates = (arr: number[]): number[] => {
  return [...new Set(arr)];
};

const firstStart = Date.now();

removeDuplicates(arr);

const firstEnd = Date.now();

// Remove duplicates from array without Set
const removeDuplicates2 = (arr: number[]): number[] => {
  const result: number[] = [];
  arr.forEach((item) => {
    if (!result.includes(item)) {
      result.push(item);
    }
  });
  return result;
};

const secondStart = Date.now();

removeDuplicates2(arr);

const secondEnd = Date.now();

console.log(`Remove duplicates from array (Set VS loop)`);
console.log(`---------------------------------`);
console.log(`Set: ${firstEnd - firstStart} ms`);
console.log(`Loop: ${secondEnd - secondStart} ms`);

Rida F'kih • Oct 25 '22

Thanks for your reply! You seem to have built a completely different solution than that mentioned in the article, the reason the function you've created is so slow is because you're iterating over each element in the array at Array#forEach, then on each item in the array you're iterating on the array again Array#includes, this effectively means that your function runs in O(n^2) time, versus the "performant" function in the article running in O(n) time.

Please note that it's not advisable to run benchmarks this way, it's best to run them using a benchmarking tool in an isolated environment.

Anyways, here is the portion of your script re-written with the function listed in the article in mind, where the nested iteration is not present.

const arrayOfRandomValues = [...Array(1000000).keys()].map(() =>
  Math.floor(Math.random() * 10000)
);

const removeDuplicates = (array) => {
  const uniqueValues = [];
  const seenMap = {};

  for (const item of array) {
    if (seenMap[item]) continue;
    seenMap[item] = true;
    uniqueValues.push(item);
  }

  return uniqueValues;
};

const start = performance.now();
removeDuplicates(arrayOfRandomValues);
const end = performance.now();

console.log("Run for-loop duplicate removal function.");
console.log(`${end - start}ms`);

The resulting output was 14.339...ms on my personal machine, but once again this is not an isolated environment.

𒎏Wii 🏳️‍⚧️ • Oct 25 '22

Array Deduplication

You can make your solution even faster by using a set instead of an object to remember the known values :D

Rida F'kih • Oct 25 '22

Do you have an example? Would love to see! Please list browser engine and environment as well! Would be much appreciated.

Yongchang He • Oct 24 '22

Thank you for sharing!

codebytesfl • Oct 24 '22

I like what you've done, but you're mainly just moving things to re-usable functions. This is generally a really nice approach and in my opinion makes things more more readable and usable. Rewriting to for loops versus built in javascript functions is a negative thing to me. Built in javascript methods and utilities such as map(), reverse(), split(), Set() and spread operators are much cleaner and most professional javascript developers can easily read and understand this.

Javascript is fast, and using a for loop for just performance reasons is negligible in most real world cases. If you have to process millions of objects in an array in client side javascript, you have bigger issues you need to be worrying about.

Rida F'kih • Oct 25 '22 • Edited

I can very well see how someone might very well come to that conclusion, my intention in the blog post was never to advise against using map, reverse, split, and Set simply based on their own merit or just because they're built-in functions. I do bring up valid use-cases for spreading a Set, and by no means forbid them as an absolute. I showcase that they absolutely can be implemented fairly, and went on to specifically highlight a situation in which they actually prevail over other methods.

Specifically in the case for the Set, for example, I mention that if you want to go with that implementation, it wouldn't hurt to leave a comment explaining how it works for developers that might not have come across a Set-spread before. Simple as that! In terms of the string reversal, which I'd guess is the one that might lead people to believe I'm advising from built-in functions for no good reason, I did say in the blog post...

In this case, we’re not off to a bad start! We could use a for-loop to get an 80% performance boost on short strings, and we might consider implementing that method if we’re reversing some chunky strings often, but in the case of the odd reversal on lightweight strings, we might just be better off with this functionality.

...which is to say, unless we're doing some odd massive string-reversals, we should be just fine with the original code, and I simply provided the more performant version "for fun," as stated in the article.

While I do touch on speed (which I think is important not to neglect) especially when working with heavy frontend libraries such as React which don't always provide a lot of room for error for newer developers, major takeaways and talking points are the naming of variables, length of lines, scan-ability of code, legibility of code, actual methods, genuine mistakes in the code, and to not prioritize code shortness over performance. By no means was the takeaway supposed to be "stop using built-in methods and start using for loops," for if that were the case, I'd be a hypocrite.

Please also note that this is a blog post about JavaScript, not exclusive to frontend development in particular, so to critique because frontend feels a little disingenuous. Regardless, I hope you were able to enjoy the article somewhat.

EDIT: Just a few minutes after my response, giving this reply a once-over I apologize if I came off aggressive or accusatory! For full posterity, I 100% agree that the changes I made in these refactor are not to be a catch-all, not always applicable, and not always necessary. As a developer, granted the majority working on the codebase agree with a style of coding, I'm game! Cheers!

codebytesfl • Oct 25 '22

Great clarification. My main point is your method of splitting code off into re-usable functions I think was the single biggest improvement to readability and elegance. I do it all the time, and I see it done all the time in enterprise apps. I think this idea should be highlighted more in general.