About linters, code quality, quality in general and quality management

Fear your desires, they can be fulfilled.
Popular wisdom.

One couple wished to get married and find eternal happiness. I blew up their car at the church right after the wedding.
One Wish Grant, film Route 60.



Another philosophical note about management, and in this case quality, consists of three parts: a very abstract, moderately abstract, concrete and separate conclusion. Contains criticism of the existing practice of applying linters.

A very abstract part about quality

First, I want to talk about quality, or rather, quality management of anything, about a product in the broadest sense, a product as a result of human activity, whether it is creating a new (writing code or a picture, designing a spaceship), cutting off excess ( , milling, selection of good fruits), or transformation (transportation, freezing, packaging, making gasoline and plastic from gas).

A good product has some signs that it is of high quality. Different products have different signs. For example, good fruit smells good, looks good, tastes good.

Now I will give a hyperbolized example, and later I will go directly to the code.

Imagine that we have a fruit shop and there is a problem, our fruit has become worse for sale, and a competitor has a queue straight. We do research and find out that visitors do not like the smell near our stalls. And the smell from the competitor's shops is like. Oh, we found a problem, the index of satisfaction with the smell! Let's decide it, there is aromamarketing, just put the automatic installation near the shelves and get a wonderful smell of apple orchard. Made. And the customer satisfaction odor index naturally smelled upwards. Only now buyers are even less.

If you look seriously, the original problem could be completely different:

1. Our competitors sell equally high-quality fruits, but they made aromamarketing before us and attracted visitors with the smell.
2. Our fruits are good, but competitor fruits are really better than ours (varieties, storage, whatever)
3. Our fruits are rotten. They just rot and stink.
4. Fruited fruits from last year that stand behind the window, and we hope that we can still sell them. And they stink from there.
5. The competitor has more range
6. The competitor has more beautifully laid out his fruit, in general, the same as ours.
7. It's stupid cheaper there
8. There the seller is beautiful, but we have a woman, Manya, who is a substitute ...

It is obvious that only in the first case will help us aromamarketing. In some, he can help, but at the same time he can disguise the real problem, and in the third one either will not take action or cause even more disgust. And oh, how often the problem is that the fruit is rotten.

In fact, when such a problem arises, it is necessary to comprehensively analyze the causes and take a case-specific solution in each particular case.

Even more hyperbolized

You have green tomatoes, and you know that only red tomatoes are sold. No need to paint tomatoes. It is good that maturation can be accelerated with ethylene . And it will be a full ripening, not painting. If it was impossible to accelerate, then it would be necessary to throw out these tomatoes and get new, already good ones.

In other words, if you are not satisfied with the quality of the resulting product, then there are problems in the chain of its production and you need to analyze and change the process, and not to paint at the output.

Well, you understand. If something smells bad, perfume will not help.

Moderately abstract part about code quality

Among the properties of good code, we find (without sorting by importance):

• consistency of style
• readability
• performance,
• extensibility
• transparency of architecture and patterns.

This is achieved primarily with the help of self-discipline and skill level of developers, as well as, when there are many developers, with the help of the code style agreement and architecture agreements (MVC, MVVM, ECS, thousands of them). The quality code appeared much earlier than the linters, any agreements and architectural patterns.
Most of the rules of the linters are purely cosmetic and solve the problem of increasing the readability of the code through the uniform use of small and local practices. The length of the lines is there, the names of the variables, const wherever there is no modification, sometimes even restrictions are imposed on the cyclomatic complexity of functions. This is not about specific rules, but about the fact that these rules are in general cosmetic, they help the code to look better. The key word herelook .

When an indicator begins to be used as a target, it loses its value as a tool.
Free interpretation of Goodhart's law .

Now let's draw an analogy with tomatoes. Ours are not mature enough. The automatic linter will tell us: “Look, you need to see the wrong color here and here.” What will the programmer do? Very often painted. And therein lies the main idea of ​​my criticism of the linter. Now I will give a specific example, and then draw a conclusion.

Concrete

PixiJS February 2, 2018 (a year ago).

A pool of requests arrives . The point is that previously a constant number of points was used to draw a curve, which is obviously not optimal. It is proposed to use a clever algorithm to estimate the length of the curve. The algorithm is not rocket science, but definitely not obvious, published in 2013 and cited with reference to the article of its author (there are problems with https). The happiness that he was generally preserved on a personal page.

It contains C code (16 lines):

floatblen(v* p0, v* p1, v* p2){
 v a,b;
 a.x = p0->x - 2*p1->x + p2->x;
 a.y = p0->y - 2*p1->y + p2->y;
 b.x = 2*p1->x - 2*p0->x;
 b.y = 2*p1->y - 2*p0->y;
 float A = 4*(a.x*a.x + a.y*a.y);
 float B = 4*(a.x*b.x + a.y*b.y);
 float C = b.x*b.x + b.y*b.y;
 float Sabc = 2*sqrt(A+B+C);
 float A_2 = sqrt(A);
 float A_32 = 2*A*A_2;
 float C_2 = 2*sqrt(C);
 float BA = B/A_2;
 return ( A_32*Sabc + 
          A_2*B*(Sabc-C_2) + 
          (4*C*A-B*B)*log( (2*A_2+BA+Sabc)/(BA+C_2) ) 
        )/(4*A_32);
};

And the following code (JS) was sent to the request pool:

/**
     * Calculate length of quadratic curve
     * @see {@link http://www.malczak.linuxpl.com/blog/quadratic-bezier-curve-length/}
     * for the detailed explanation of math behind this.
     *
     * @private
     * @param {number} fromX - x-coordinate of curve start point
     * @param {number} fromY - y-coordinate of curve start point
     * @param {number} cpX - x-coordinate of curve control point
     * @param {number} cpY - y-coordinate of curve control point
     * @param {number} toX - x-coordinate of curve end point
     * @param {number} toY - y-coordinate of curve end point
     * @return {number} Length of quadratic curve
     */
    _quadraticCurveLength(fromX, fromY, cpX, cpY, toX, toY)
    {
        const ax = fromX - ((2.0 * cpX) + toX);
        const ay = fromY - ((2.0 * cpY) + toY);
        const bx = 2.0 * ((cpX - 2.0) * fromX);
        const by = 2.0 * ((cpY - 2.0) * fromY);
        const a = 4.0 * ((ax * ax) + (ay * ay));
        const b = 4.0 * ((ax * bx) + (ay * by));
        const c = (bx * bx) + (by * by);
        const s = 2.0 * Math.sqrt(a + b + c);
        const a2 = Math.sqrt(a);
        const a32 = 2.0 * a * a2;
        const c2 = 2.0 * Math.sqrt(c);
        const ba = b / a2;
        return (
                (a32 * s)
                + (a2 * b * (s - c2))
                + (
                   ((4.0 * c * a) - (b * b))
                   * Math.log(((2.0 * a2) + ba + s) / (ba + c2))
                  )
               )
               / (4.0 * a32);
    }

The code is designed in full accordance with the settings of the linter. There are descriptions of all parameters, a link to the original algorithm, a bunch of const, in accordance with the no-mixed-operators linter requirement: 1 are placed in parentheses. Even for performance, api is made not with an object, but with separate parameters, so it is really usually better in JS.
There is one problem. This code does complete garbage. (An attempt to calculate in Russian the expression fucked up, which is fully used in Western publications to express the degree of the problem and seems to be appropriate).


A huge length is returned, and many points are allocated to it, it’s good that there was a restriction from above, and according to it it worked. Previously, this mode was disabled by default, but then turned on for everyone (because of another bug, by the way). Fix already by the way . I did not contact the author of the commit and did not ask him why he decided to put the brackets, but I feel that he launched the linter, the configuration file of which is already in PixiJS. This linter told him, your code is bad, because there are not enough brackets in it, add brackets. The option "no-mixed-operators" says that you have no right to write

2*2+2*2

because it can lead to poor readability. Someone created this option, then someone included it in the project, which suggests that many people consider it useful.

Conclusion

I do not want to say that the linters are evil, but I consider them to be such an application. We (in the sense of humanity) were able to automate the detection of only a small part of the signs of good code, mainly cosmetics like the placed brackets. Linters are good as tools for analyzing code quality, but as soon as we put the requirements of the linter in the framework of the mandatory requirements, we get this same requirements. We do not acquire anything except compliance with the requirements. It’s like putting a camera on an envelope with tomatoes and sending it in for painting all that are not red enough. While we did not give the developer a tool for assessing the quality of the appearance of the code, he could send bad code, and we could see it.Now the bad code will be better disguised. It will mimic a good one, because all the external signs of a good code are on it. And we will lose the linter as an assessment tool, because the whole code corresponds. We had an assessment tool, but now it is not there, but the code with parentheses, though not there sometimes, but these are details. Total linter I think a cool tool, but only if compliance with the requirements does not become the goal .

And yes, here we can say that there are no tests, that you do not need copy-paste code, that it is stackOverflow development, that you don’t insert code that you don’t understand. This is all yes. And this is a sign of bad code. But linter helped make it look visually similar to everything else in the project. But a linter will never check if you understood what you wrote.

In other words, I think that the correct use of a linter is to regularly launch it on a project with a lead and an assessment of how and what goes. Well, the rules of the type more brackets to God brackets, in principle, I consider harmful. When we see that someone commits a code of poor quality, then it is worth understanding why he does it and solve this problem at a deeper level. Naturally formatting the code with your hands is not necessary, I in every way welcome autoformers, but as long as they don’t touch the semantic part of the code in any way. If we use a linter to force a person to bring the code to the standards, then we will essentially color the green tomato red. And it will be even harder to understand that it is actually green. What to do in open projects with a bunch of different people, the question is more complicated, but even here you can think what to do.

It should be said that my attitude towards the linter was formed long ago, more than three years ago, but I could not find a suitable example in practice when the linter played a cruel joke. And so I found him. The fact that I’ve been looking for him for so long says that the problem is not so big, or how difficult it is to notice the negative effect, but I think this note will be useful. Remember, a linter is a tool, and as any tool it can be used to harm and to the good, and as with any tool you can sometimes cut yourself.