Discussion on: Preaching 'Clean Code' is Lowering the Quality of Developers

[tq-bit]

TL:DR

You're right, if you preach about clean code instead of thinking through proper solutions, you're off worse than getting stuff done. But that doesn't mean you should be careless about aesthetics. Code is read by people and interpreted by a machine. Nobody likes to read a badly written article, why would devs like to read badly written code? Even if they have to, don't you think it'd lower their performance more than a clean codebase?

---

There are a few things I find very enlightening here and some I just can't agree with.

We would actively seek to dumb down code to the point where it is more like an introductory reader for a five year old, than an elegant, nuanced work by a master author

Isn't this a good thing? The best books I've ever read were the ones I could easily digest while learning a lot (e.g. Books by Kahnemann or Carnegie). You're right with one thing - it takes time, patience and practice to write code that just works, it just doesn't stop there. If I was presented two codebases

1. Works, but looks ugly
2. Looks beautiful, but has tons of bugs

I'd always scrap 2 and move on with 1. Refactoring imo is more fun (and often less time consuming) than debugging.

Learning in this way will bring a much fuller understanding - resulting in much more competent, and - more importantly - creative developers.

Again, not saying being creative is a bad thing per se. But try to imagine

• You're lead dev of a 5-person team
• 2 fe, 2be, 1qa guys.
• 1be dev, 1qa dev quit because they received a better job offer. Vacancies are opened to be filled within the next 3 months
• Your customer wants a new feature by yesterday
• Your manager wants to deploy more features faster (= optimise pipe)
• Nothing is documented, half of the codebase knowledge is gone with be dev #2

Taking over responsibility, you'll jump into the codebase trying to grasp what's going on. You see one module by dev#2 looking like that (pseudocode to extract mandatory build steps for a database schema, based on a true story):

export function build(cmd, cd) {
  const array = [];
  let ct = 0;

  for (let k in cmd) {
    cd.forEach(item => {
      if(cmd[k].step === item.step) {
        ct = count(count)
        array.push(cmd[k].step);
      }
    })
  }
  console.log(`Built ${ct} times`)
  return array
}

function count(abc) {
  return abc +1
}

In another universe, your other, but slightly luckier you, is confronted with the same problem. With a difference in the codebase:

type DatabaseBuildStep = {
    step: string;
    handler?: () => void;
};

type DatabaseBuildStepMap = {
    [key: string]: DatabaseBuildStep;
};

interface DatabaseBuildTraits {
  appliedSteps: number;
    build(
        availableSteps: DatabaseBuildStepMap,
        mandatorySteps: DatabaseBuildStep[]
    ): DatabaseBuildStep[];
}

export default class Database implements DatabaseBuildTraits {
    public appliedSteps: number;

    public build(availableSteps: DatabaseBuildStepMap, mandatorySteps: DatabaseBuildStep[]) {
        let buildSteps: DatabaseBuildStep[] = [];
        for (let key in availableSteps) {
            const buildStep = this.extractMandatoryStep(availableSteps[key], mandatorySteps);
      if(!!buildStep) {
        buildSteps.push(buildStep);
      }
        }
    console.log(`Applied ${this.appliedSteps} building steps`)
        return buildSteps;
    }

    private extractMandatoryStep(
        availableStep: DatabaseBuildStep,
        mandatorySteps: DatabaseBuildStep[]
    ): DatabaseBuildStep | null {
    if(mandatorySteps.includes(availableStep)) {
      this.incrementSteps();
      return availableStep;
    }
    return null
  }

  private incrementSteps() {
    this.appliedSteps ++
  }
}

There's a few obvious differences:

• The 2nd codepart is more than 2x as long as the first
• The TS code is verbose and (probably) bloated
• Both snippets are far from perfect. I mean, we have deadlines to keep after all.

What makes the second, class based approach superior to the module implementation is:

• It follows a logical top-down order of responsibilities (one function doesn't do it all)
• It conveys meaning by its Types and Interface traits
• It's readable by a human
• The TS compiler complains if returned variables looks fishy
• It's less prone to bugs and can easily be interpreted by other devs

Both times, the code probably works. And some people might say: "What's wrong with code 1? It does the job, doesn't it?".

Sure. But be honest. Put yourself into the world of the poor lead developer who has to jump into the breach whenever shit hits the fan. Once you're confronted with the code: Which approach would you prefer - Quick & Dirty OR well thought & aesthetic?

[𒎏Wii 🏳️‍⚧️]

I'd disagree with your assertion that the second snippet is (slightly) better; it is, in my opinion, more complex in its structure, whereas the first snippet only seems more complex because of the poorly named variables and some sub-optimal choices of syntax.

This sort of problem is usually easy to clean up by just renaming the variables to what they represent (cmd → commands, ct → counter, etc.) and tweaking the code to use more readable syntax (for ... in → for ... of, not mixing for loops with forEach for a nested loop, etc.)

Meanwhile the second snippet of code obfuscates its intention much more deeply with its program structure. A function is hidden behind a stateless class (instant red flag), trivial actions (increasing a counter) are extracted into separate subroutine, but it hides these flaws by having more descriptive variable names, so it seems "friendlier" to the reader.

[Jon Randy 🎖️]

I think you may have misunderstood me somehow. In no way did I suggest being careless about aesthetics

[tq-bit]

I wasn't so sure here, might as well be a misinterpretation from my side.

On the one hand, you mention 'elegant, nuanced work', on the other hand, to quote:

it used to be that you just had a huge tub of bricks, and you used your own ingenuity and imagination to firstly work out how all the pieces 'work' and fit together

I'd interpret this statement as a call to action for a trial and error approach. It works fine. Just in my experience, just as often it leads to the 1st scenario I described above (working, but 'ugly' code). Which still isn't a problem until the first change request hits.

Since I have an econ/business'ish background, I've got two hearts beating in my chest. Be diligent AND done in time/budget. Which is extremely hard to achieve. Uncle Bob got me with one statement though.

If you try to code fast, on the long run you'll always be slow

[Jon Randy 🎖️]

Yup, really seems like we're not on the same wavelength

[toxicsmurf]

what did I just read?