1. 40
  1.  

  2. 21

    The author doesn’t mention a more important problem with the new, ‘clean’ code: it replaces repetition with lots of new local idioms, making it much harder to read.

    I think the intuition of trying to prevent repetition was correct, but it seems to me that he could simply have extracted some duplicate math into a separate function instead.

    1. 17

      This is my immediate reaction to this entry too. They ‘cleaned’ their code by introducing ad hoc abstractions which a future reader could not escape from. They didn’t quite remove repetition, they traded it for a more complicated structure. Which now puzzles them that it is cognitively more expensive than their previous code.

      This is the problem with blindly following mantras. People follow mantras blindly after some guy in some fancy blog post or book says it is a good idea, rather than understanding why it is [potentially] a good idea. The tittle hints it as it refers to a book that replaced common sense with a cult. Notice the sentimental attachment in ‘goodbye’.

      They should have resourced to a construct that any possible reader already is familiar with: a function, providing a free abstraction. Of course, the problem they had at hands is essentially side effecting the system, which challenges this strategy.

      1. 6

        Yeah. Simple and dumb code using universally known abstractions (function, list, map) is often better that a clever custom made one.

        There was a consultant coming once to teach us to be a better coder. The style was much “I read a lot of book, but haven’t had a lot of experience working in enterprise”. At some point he made us do a Kata exercises where we modeled a system triaging patient in a hospital. The point he was trying to made was that we should extract an abstraction for a priority queue.

        — Was it better once you extracted you queue?
        — I didn’t extract a queue.
        — Then your code doesn’t reflect an important abstraction in the domain.
        — Honestly, I think my code is fine and I would be perfectly happy to find it in the wild. It use Java Lists and Maps, has well named variable and is well chopped into reasonably sized function.
        — But you are missing a concept.
        — Honestly, for that kind of code, I always found that the way it is coded now is the way I would regret the least later.
        — it’s not domain driven
        — Maybe not. But it’s easy to read, work correctly and easy to changes, and that is what I ever cared about.
        — you may have trouble later because the missing abstraction will make people add code haphazardly.
        — They would do it no matter what in my experience, and we have code review for that. What about you? Do you ever found that it ever help?
        — Abstractions help a lot actually. They allow you to…
        — Yes, yes, but can you give a me real and concrete experience where YOU created such an abstraction in a production code base and list me some REAL problem you found it avoided?

        At this point, the conversation stopped being productive because one of us could not really speak from a real tangible experience.

        1. 3

          “Bro have u even shipped” is a valid rhetorical device to end a conversation, it might have been more educational if the consultant showed their abstraction. I would like to be that guy and point out that you are both right, I just don’t want to work at the place that embraces the consultant’s techniques.

          I had a similar experience with a Design Pattern consultant, he even had a book. When I asked about a systemic pattern of flaws in code bases he drew a complete blank. No concept of antipatterns. I don’t mean in the sense of X bad, Y good, but more of when one goes down path Gamma, you are implicitly signing up for G, H and K.

          Humans are crazy full of cognitive blind spots, experts even more so. Lots of folks are in do loops when they would be better served with a why loop.

      2. 16

        It seems the author has a different concept of “clean”. To me, “clean” is “less complicated” rather than “less duplicated”.

        1. 3

          I think that “clean” is not a good word to use because it is subjective and ambiguous. It is better to ask

          • “Do you understand the code?” => “Can I make it easier to understand?”
          • “Is it correct? Can you determine easily that it is correct?” => “Can I make it easier to verify?”
          • “Is it fast enough?”…
        2. 16

          It was a late evening. My colleague has just checked in the code that they’ve been writing all week. The code worked. It wasn’t clean.

          Author rewrites code in one night that someone else has been working on all week based on some subjective measurement of “clean”. The boss was right to have the 1-on-1, but it has less to do with “code cleanliness” than basic respect for teammates. This type of behavior belittles the work of your teammates, makes you look like an prima donna asshole, and starts people polishing resumes because it creates a horrible work environment.

          1. 13

            The author clearly already states as much in the article:

            I see now that my “refactoring” was a disaster in two ways:

            Firstly, I didn’t talk to the person who wrote it. I rewrote the code and checked it in without their input. Even if it was an improvement (which I don’t believe anymore), this is a terrible way to go about it. A healthy engineering team is constantly building trust. Rewriting your teammate’s code without a discussion is a huge blow to your ability to effectively collaborate on a codebase together.

            So I don’t really see a point in discussing it further, certainly not without adding some substantial contribution to the debate.

            1. 4

              I’m going to be controversial and say that this is a problem. Either a single person owns the code and has to review all changes or it is owned by a community and can be modified by anyone in the direction of better.

              1. 3

                The issue isn’t code ownership, it’s someone staying late to rewrite working code, based on a subjective metric of “better code”. If you’re going to stay late, it’d better be for a contractually obligated “must-hit” deadline, or because of a major issue in production.

                1. 1

                  I dunno - if someone wants to stay late and work on something, I’m not particularly interested in policing that.

                  1. 2

                    In the environments where I’ve worked, working late sets a bad precedent, and it eventually boils up to management complaining, “Why isn’t Joe (the only one not peer pressured into working late) working late tonight!?” Especially when used to complete non-essential work, it can lead to management failing to prioritize because there’s “more hours in the day.” I’ve been salaried working 80+ hours a week because of this type of mess.

                2. 3

                  Years later, sure. Team and human factors always matter in the immediate time frame, regardless of future ownership constructs.

                3. 3

                  I’m not so sure. Not talking about this case, but first of all it doesn’t matter how long person A spent on the code and if person B objectively improves it… At least I have zero problems with people improving my code, all that matters is the result at the end, I’m not married to my code. That said, I don’t do this to other people’s code because they might react like you did :) I would have a stern talking to the people who let me work on something for a week and not giving any input how I could speed it up. I usually do ask for help if tasks seem huge and while it doesn’t help anyone if the same people do their special thing very quickly all the time, I suppose pairing for a few hours might have helped get better code quicker.

                  1. 1

                    first of all it doesn’t matter how long person A spent on the code and if person B objectively improves it…

                    This isn’t why the author “improved it”. The author said it wasn’t “clean” which isn’t some sort of objective measurement.

                    not giving any input how I could speed it up

                    The author didn’t do it any faster. It was rewritten to match the stylistic tastes of the author, late at night on the day the it was originally submitted, instead of the author going home for the day.

                    I don’t do this to other people’s code because they might react like you did

                    I’ve had this happen to me I don’t care. I’ve seen this happen to other people too and it was just another notch in them moving closing to the door and eventually leaving.

                    1. 1

                      Author rewrites code in one night that someone else has been working on all week based on some subjective measurement of “clean”

                      I was referencing this part of your post, you repeated it, I thought it was to underline that fact.

                      That’s what I meant. It’s possible that coworker A who has intricate knowledge can do it in a fraction of the time. In this case it sounded like the author could have done it faster, but I may have misread - but it’s moot if I was misreading your sentence.

                4. 9

                  The correct process: create a merge request and put the original author as a reviewer.

                  Anyway the change looks much more like a process smell then a code smell.

                  1. 9

                    This should have been the moral of the story, the author rewrote someone’s code and merged it to master in the middle of the night. It’s a short sighted and mean thing to do, and probably the reason the author was told to revert their work. This isn’t a story about clean code, it’s a story about working with teammates.

                    1. 2

                      Correct-er process: privately speak to the original author and mention that you would like to rewrite and give them an overview of how & why. Sometimes they may be able to talk you out of it!

                    2. 3

                      I have a very similar story to share. In the context of testing, and abstraction does have a cost. In our case, the lesson was that a language allowed only controlled amount of abstractions was better than a general language.

                      1. 2

                        Very interesting. Most of the time, people talk about using non-Turing Complete to provide restrictions that facilitate machine analysis. Your observation of restricting people’s habits might be even more important. It’s similar to why we include strong, type systems in some languages. Just applying that to the size and expressiveness of the language in this case.

                      2. 3

                        “Clean Code” is also a dedicated term for a dedicated coding technique. And when you learn it, you learn that Refactor is one step in the cycle : Red -> Green -> Refactor.

                        • If the tests were green after the Refactor, then the code was as good as before the refactor. It was not as bad as the author attempts to make us think. It was working, as good at the previous code.

                        • If the code was more readable after the Refactor, then the code was INDEED more readable after the Refactor.

                        • If as the author suggests, some times later, the abstraction would have become a hurdle, because the reality often does not comply with our expectancy, it would have become obvious a new Refactor was needed. And , at a stage where all tests were green, the needed Refactor would have been to remove the added abstraction, or choosing another one, another pattern, another function… And the code would have become more readable than before (before = the moment when complexity made the abstraction become a hurdle getting in the way).

                        “The code is written once, and read a hundred times.” Of course there are projects where this statement is not true. But these are project that are not actively maintained and modified.

                        The trade-off to chose has to take into account: the number of times the code is read multiplied by the time it takes to understand the code, and also the probability to introduce a bug when the time to understand the code is constrained.

                        Readibility matters. But readibility also evolves as the project evolves.

                        The author’s advice not to be a zealot is a nice advice. The real Zen is outside the Zen.