Side effects are any observable change to the state of an object. Some people might qualify this by saying that side effects are implicit or unintended changes to state.

Mutator methods, or setters, which are designed to change the state of an object, should have side effects. Accessor methods, or getters, should not have side effects. Other methods should generally try to avoid changing state beyond what is necessary for the intended task.

Why are these guidelines best practices though? What makes side effects so bad? Students have asked me this question many times, and I've worked with many experienced programmers who don't seem to understand why minimizing side effects is a good goal.

For example, I recently commented out a block of my peer's code because it had detrimental side effects. The code was intended to add color highlighting to log entries to make his debugging easier. The problem with the code was that the syntax highlighting xml was leaking into our save files. He was applying the highlight to a key, then using that key both in the log and in the save file. Worse still, he wrote this code so that it only occurred on some platforms. When I was debugging a cross platform feature, I got very unpredictable behavior and ultimately traced it back to this block.

This is an example where code was intended for one purpose, but it also did something else. That something else is the side effect, and you can see how it caused problems for other developers. Since his change was undocumented and uncommented, I spent hours tracking it down. As a team, we lost significant productivity due to a side effect.

Side effects are bad because they make a code base less agile. Side effects cause bugs that are difficult to find, and lead to code that is more difficult to maintain.

Before I continue, let me be clear that all code style guidelines should be broken sometimes. For each situation, a guideline or design pattern may be better or worse, and I recognize that we are always working in shades of gray.

Generally, a block of code should be written for one purpose. If it is a method, then it should do one thing. If another thing needs to be done with an object, then that should be encapsulated in another method.

Here's a hypothetical example that I've seen played out hundreds of times in my career.

A class needs to do task X. A programmer may write a method to do task X, but he accidentally includes logic that also does task Y. Later, he may see that he needs to do task Y all alone. So he writes a method to do task Y. That's where the problem is compounded.

Later still, the definition of task Y changes. So another programmer has to rewrite task Y. He goes to the class, changes the method for task Y alone, does a few quick tests, and proceeds on his merry way.

Then mysterious bugs start occurring. QA can't really track them down to one thing because they only occur sporadically after task Y. Finally, it takes many man-hours to remove task Y from the method for task X.

In this example, the side effect led to code duplication, which led to trouble when updating the code, which led to bugs that cost many hours to track down. The fewer side effects you introduce, the easier your code will be to maintain.

These two examples show how side effects can derail development and why they are so inimical. We all write code with side effect occasionally, but it's our job to figure out how to do it in a way that doesn't make the code more difficult to maintain.

Duplicating code is a bad thing. Any engineer worth his salt knows that the more you repeat yourself, the more difficult it will be to maintain your code. We've enshrined this in a well-known principle called the DRY principle, where DRY is an acronym standing for Don't Repeat Yourself. So code duplication should be avoided at all costs. Right?

At work I recently came across an interesting case of code duplication that merits more thought, and shows how there is some subtlety needed in application of every coding guideline, even the bedrock ones.

Consider the following CSS, which is a simplified version of a common scenario.

.title {
  color: #111111;
}
.text-color-gray-1 {
  color: #111111;
}

This looks like code duplication, right? If both classes are applying the same color, then they do the same thing. If the do the same thing, then they should BE the same thing, right?

But CSS and markup in general presents an interesting case. Are these rules really doing the same thing? Are they both responsible for making the text gray? No.

The function of these two rules is different, even though the effect is the same. The first rule styles titles on the website, while the second rule styles any arbitrary div. The first rule is a generalized style, while the second rule is a special case override. The two rules do fundamentally different things.

Imagine a case where we optimized those two classes by removing the title class and just using the latter class. Then the designer changes the title color to dark blue. To change the title color, the developer now has to replace each occurrence of .text-color-gray-1 where it styles a title. So, by optimizing two things with different purposes, the developer has actually made more work.

It's important to recognize in this case that code duplication is not always code duplication. Just because these two CSS classes are applying the same color doesn't mean that they are doing the same thing. In this case, the CSS classes are more like variables than methods. They hold the same value, but that is just a coincidence.

What looks like code duplication is not actually code duplication.

But… what is the correct thing?

There is no right answer here. It's a complex problem. You could solve it in lots of different ways, and there are probably three or four different approaches that are equally valid, in the sense that they result in the same amount of maintenance.

The important thing is not to insist that there is one right way to solve this problem, but to recognize that blithely applying the DRY principle here may not be the path to less maintenance.