+6

Goodbye, Clean Code

It was late in the evening and my colleague had just finished writing the code for the week. We were working on a graphics editor canvas and they had implemented the ability to resize shapes like rectangles and ovals by dragging small handles at their edges.

The code worked, but it was repetitive. Each shape had a different set of handles and dragging each handle in different directions changed the shape's position and size in a different way. If the user held down the Shift key, we had to make sure to keep the proportions of the shape while resizing. This required a lot of math.

The code looked something like this:

let Rectangle = {
  resizeTopLeft(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeTopRight(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeBottomLeft(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeBottomRight(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
};

let Oval = {
  resizeLeft(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeRight(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeTop(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeBottom(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
};

let Header = {
  resizeLeft(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeRight(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },  
}

let TextBlock = {
  resizeTopLeft(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeTopRight(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeBottomLeft(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
  resizeBottomRight(position, size, preserveAspect, dx, dy) {
    // 10 repetitive lines of math
  },
};

I was feeling frustrated because I had to do a lot of math that was very similar. Most of the repetition was between similar tasks, like resizing the left side of an oval or a header. There was also some duplication between tasks for different shapes, like ovals and rectangles.

I had an idea to group the code together to remove the duplication:

let Directions = {
  top(...) {
    // 5 unique lines of math
  },
  left(...) {
    // 5 unique lines of math
  },
  bottom(...) {
    // 5 unique lines of math
  },
  right(...) {
    // 5 unique lines of math
  },
};

let Shapes = {
  Oval(...) {
    // 5 unique lines of math
  },
  Rectangle(...) {
    // 5 unique lines of math
  },
}

and then composing their behaviors:

let {top, bottom, left, right} = Directions;

function createHandle(directions) {
  // 20 lines of code
}

let fourCorners = [
  createHandle([top, left]),
  createHandle([top, right]),
  createHandle([bottom, left]),
  createHandle([bottom, right]),
];
let fourSides = [
  createHandle([top]),
  createHandle([left]),
  createHandle([right]),
  createHandle([bottom]),
];
let twoSides = [
  createHandle([left]),
  createHandle([right]),
];

function createBox(shape, handles) {
  // 20 lines of code
}

let Rectangle = createBox(Shapes.Rectangle, fourCorners);
let Oval = createBox(Shapes.Oval, fourSides);
let Header = createBox(Shapes.Rectangle, twoSides);
let TextBox = createBox(Shapes.Rectangle, fourCorners);

The code I changed was half the size it was before and all the duplication was removed. It was much cleaner and easier to understand. If we wanted to change the behavior for a particular direction or shape, we could do it in one place instead of having to update methods in multiple places. I was very proud of my work and went to bed after I saved my changes to the master version.

The Next Morning

… did not go as expected.

My boss asked me to undo a change I had made during a private conversation. I was shocked because I thought my change was an improvement over the old code. Even though I was unhappy about it, I changed it back and eventually realized that my boss was right.

It’s a Phase

Many of us have gone through a period of time where we focus a lot on writing "clean code" and getting rid of any duplicated parts. When we don't feel sure about our code, it can be tempting to link our self-esteem and pride in our work to something that can be measured, like a set of strict rules for formatting, a specific way of naming things, a certain way of organizing files, and no duplication.

It is not possible to use a computer to automatically remove duplicated code, but it does become easier with practice. You can usually tell if there is less or more duplication after making changes. This makes it feel like you are improving the code in some way. Unfortunately, it can also make people feel like they are not good at writing clean code, which can be as powerful as any kind of self-deception.

Once we understand how to make things simpler by breaking them down into smaller parts, it is easy to get carried away and create abstractions for any code that looks similar. After a while, we start to see patterns everywhere and it feels like we have a special ability. If someone tells us that abstraction is a good thing, we will believe them and start to criticize others for not writing code in a neat and organized way.

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

  • I didn't talk to the person who wrote the code before I changed it and submitted it. Even if I thought it was an improvement, this was a bad way to do it. A good engineering team needs to trust each other. Changing someone's code without talking to them first damages the team's ability to work together on the same code.
  • Nothing comes without a cost. I gave up the ability to change requirements in exchange for less duplication in my code, but it was not a good decision. For example, we needed different behaviors for different shapes and handles, which would have made my code much more complicated. However, with the original "messy" version, making these changes was very easy.

Are you suggesting that I should not write "dirty" code? No. I suggest that you think carefully about what you mean when you say "clean" or "dirty". Do you feel angry, righteous, beautiful, or elegant? Are you sure that you can name the specific engineering results that come from these qualities? How do these qualities affect the way the code is written and changed?

I didn't think carefully about any of those things. I was more focused on how the code looked rather than how it would work with a group of people.

Coding is a process of learning and growing. Think about how much you have improved since you wrote your first line of code. It was probably exciting to see how breaking a problem down into smaller parts or reorganizing a group of instructions can make complicated code easier to understand. If you take pride in your work, you may be tempted to keep your code neat and organized. Try it for a while and see how it goes.

Do not only focus on writing clean code. Clean code is not the ultimate goal. It is a way to make sense of the complicated systems we work with. It is a way to protect ourselves when we are not sure how a change will affect the code, and we need help in an unfamiliar situation.

Write your code in a clear and organized way, but don't get too attached to it. Let clean code guide you. Then let it go.

Mình hy vọng bạn thích bài viết này và học thêm được điều gì đó mới.

Donate mình một ly cafe hoặc 1 cây bút bi để mình có thêm động lực cho ra nhiều bài viết hay và chất lượng hơn trong tương lai nhé. À mà nếu bạn có bất kỳ câu hỏi nào thì đừng ngại comment hoặc liên hệ mình qua: Zalo - 0374226770 hoặc Facebook. Mình xin cảm ơn.

Momo: NGUYỄN ANH TUẤN - 0374226770

TPBank: NGUYỄN ANH TUẤN - 0374226770 (hoặc 01681423001)

image.png

Resource


All rights reserved

Viblo
Hãy đăng ký một tài khoản Viblo để nhận được nhiều bài viết thú vị hơn.
Đăng kí