Programmers like compact code. If it's done correctly, then it should be easy to read and shouldn't contain any parts that make you think about it more than necessary.
For example::
// Real code from the open source Hexlet project
const programImports = source.program.body
.filter(item => item.type === 'ImportDeclaration')
.filter(item => item.source.value.startsWith('hexlet'));
// Entire code is here https://github.com/Hexlet/hexlet-exercise-kit/blob/master/import-documentation/src/index.js
Those already familiar with the concept of filtration understand it clearly enough not to waste time on further polishing it.
But sometimes wanting to make the code more compact leads to the opposite. Its readability drops and refactoring becomes more difficult. Also, the author of the code is not always aware of this. A programmer working on the problem right now can understand almost any solution, no matter how bad. At this point, the programmer has all the details in their head and finds it easy to explain the code. The same cannot be said for others who are reading it. Moreover, after a day, two days, or a week, even the author will find it difficult to understand.
Hexlet projects are an endless source of code examples for this article. I have chosen a few of the most interesting ones and I will analyze them below.
// Many programmers sincerely believe this code is good,
// because there are no variables here But that's not right.
compareFiles(JSON.parse(fs.readFileSync(fullPathToFile1)), JSON.parse(fs.readFileSync(fullPathToFile2)))
This example is still clear and does not contain complicated logic, but it is already difficult to grasp. Why? The large number of nested calculations. Calling a function is complicated. The more calls, the harder it is to match them together in your head. The problem is exacerbated by the fact that the result of the call is not always obvious. So you have to keep track of all parts of the expression in your head at once.
The easiest way to make code understandable, odd as it may seem, is to create variables (or constants) with descriptive names (assuming that the functions are already named well). Then our code will turn into this:
const config1 = JSON.parse(fs.readFileSync(fullPathToFile1));
const config2 = JSON.parse(fs.readFileSync(fullPathToFile2));
compareFiles(config1, config2);
It's now easier to explain the code because the big task has been turned into several smaller ones. What we were dealing with became clearer (it's the configs!). As a bonus, debugging will be simpler because you can see what lies in the intermediate variables. Can it be improved further? Yes, nested calls should always make you want to get rid of them. Especially if the nested calls produce side effects.
For myself, I derived what I call "the rule of three challenges". It tells you to split expressions containing three or more nested calls:
f(f2(f3())); // needs breaking up
f(f2()); // probably doesn't need breaking up (or maybe it does)
FYI. The pipeline mechanism is the best way of getting rid of nested calls. Thanks to babel, it's also in JS.
Another example:
expect(genDiff(first, second, format)).toBe(fs.readFileSync(`${__dirname}${result}`, 'UTF-8').trimRight());
This expression is already more complex and analyzing it requires mental effort. Especially the second part, with toBe. This is where the three calls and interpolation lie.
Let's refactor this code:
const fixturePath = path.join(__dirname, result);
const expectedValue = fs.readFileSync(fixturePath, 'UTF-8').trimRight();
const actualValue = genDiff(first, second, format);
expect(actualValue).toBe(expectedValue);
Yes, there are 4 lines of code instead of one, but in this case more is better. This code does not add any new logic or increase the complexity, instead it makes the code self-documenting.
Another example:
state.formStatus = ((validator.isURL(value) && (state.rssFlows.some(rssFlow => rssFlow.url === value) === false)) ? 'valid' : 'invalid');
The expression is on the verge of an error. It can be read, but the second condition in the predicate is too complex. I'll leave it to you to figure out on your own. Show an example of how you would split this code in the comments.
One last example:
const urls = _.flatMap(['a', 'link', 'script', 'img'], item => $(item).map((i, el) => $(el).attr(attributes[item])).get().filter(el => el[0] === '/' && el.length > 1));
Try to understand what's put in what and what's called from what.
