Refactoring is stupid !

The following is a reworked and anonymised transcript of a discussion I had a short while after I published one of my last article.

I don’t really look like this…

Thomas, there you are. I just finished reading your article and I think it’s stupid ! Dumb, dumb dumb ! That refactoring you are talking about is dumb. Here, let me explain it to you. Let’s take this piece of code for example:

public double computeElectricalResistance() {
  ParticularElectricApi particularElectricApi = new ParticularElectricApi();
  double tension = particularElectricApi.readTension();
  double current = particularElectricApi.readCurrent();
  return tension / current;
}

I like this code ! It’s easy to understand, concise and clear. It is the very definition of Clean Code. Don’t you agree ?

Hey Jean-Michel ! Indeed, this code looks concise and clear to me but it is definitly not clean. I can see static dependency issues along with some primitive obsession. Can you explain to me what is bothering you with what I wrote in this context ?

Well, you said that here, I should introduce an interface, let’s say IElectrical and have a concrete implementation ParticularElectric that I can then inject into this code. Can’t you see the amount of code doing that would add to the code base ? More code means more bugs. It’s programming 101. That’s as simple as that.

Oh dear god, no ! I would never add an interface IElectrical. Instead in this case, I would probably prefer adding two interfaces: one for Voltmeter and one for Ammeter with their corresponding implementation ParticularElectricVoltmeter and ParticularElectricAmmeter respectivly. That sounds better to me. Ideally, I would thrive to achieve something like this:

public Resistance computeElectricalResistance() {
  Tension tension = voltmeter.readTension();
  Current current = ammeter.readCurrent();
  return Resistance.createUsingOhmLaw(tension, current);
}

What. The. Hell ??? Now you tell me I would have to add two interfaces ??? That’s insane ! That’s twice the amount of code, which means twice the number of bugs ! And I’m not even talking about those Tensionand Current classes…

We will leave the discussion about Primitive Obsession for later. For now, why do you think there would be more bugs in this code than what you came up with ?

Because there is more code. The greater the number of lines is, the higher the probability of introducing a bug is as well.

Ok, now I get where you’re going. Now tell me, what do you spend the most time doing between adding new code and changing existing code ?

Changing existing code, obviously. Even when adding new code, because we work iterativly, we end up always changing the code we just wrote.

Right ! I agree with you. So, what we should focus on primarily between reducing the probability of adding a bug when writing new code versus changing already existing code ?

Limiting the probability of introducing a bug when changing code should be the priority, logically. But I don’t see where you’re going with this…

Have some patience, we are almost there. For now, let’s go back to your example. We received a new feature request from a customer. They are not satisfied with the Ammeter provided by the ParticularElectric company because the values read are too volatile. Instead, they want to replace it another Ammeter provided by the TeslaWasBetterThanEdison Corporation. However, because the customer don’t plan to upgrade all its systems, the code should be compatible with both Ammeters. How would you do it ?

I guess I would change it to something like this:

public double computeElectricalResistance() {
  ParticularElectricApi particularElectricApi = new ParticularElectricApi();
  double tension = particularElectricApi.readTension();
  double current;
  if (useTeslaWasBetterThanEdison) {
    TeslaWasBetterThanEdison teslaWasBetterThanEdison = /* instanciate API here */
    current = teslaWasBetterThanEdison.current()
  } else {
    current = particularElectricApi.readCurrent();
  }
  return tension / current;
}

Alright, that would work. Now, do you think your code is still easy to understand, concise and clear ?

Well, no…

Now, let’s go back to what I proposed. What needs to change in order to handle this new Ammeter ?

Mmmmmh… Well, nothing. I mean, we just have to change somewhere the injectied instance of Ammeter to a TeslaWasBetterThanEdisonAmmeter but that’s it.

Exactly ! Because we decoupled how we read current and tension from what we do with them, we made the code much more independant from the detail of how we fetch those values. You see, while I agree your code was easy to understand, concise and clear, a simple functional changed that. It was not robust to change.

Yes, Clean Code should be easy to understand, but all code that is easy to understand is not Clean. Clean Code is also easy to change. So, when writing code, you always have to wonder what changes and for what reasons. Put together the things that change for the same reason and split the things that change for different reasons (like how we read the current value and what we do with it). And that, my friend, is called the Single Responsibility Principle, the first of the SOLID principles.