What is gold plating and why you should avoid it

I didn’t know what “gold plating” was until a senior engineer called out on one of my code changes and recommended that I “stop gold plating.” I was clueless about what he meant, so I went to talk to him. This meeting ended up being a memorable lesson in my software engineering journey.

Wikipedia defines gold plating as: “the phenomenon of working on a project or task past the point of diminishing returns.” While the article talks about gold plating in the context of project management, the same phenomenon occurs in software development under a more familiar name: unnecessary refactoring.

The feedback I got from the senior engineer was that he noticed a pattern where I continued working on code that was already finished. I was polishing tests without covering new scenarios, changing perfectly fine variable or method names, or making functions slightly shorter.

I felt offended – I was making the code better!

How could a senior software engineer not see it?

How could they be against improving code?

So, he asked me to explain how my changes improved the code. I couldn’t. Only then did I realize he was right. I had to admit the new code was a bit different, but it wasn’t objectively better.

But then he went further and asked me how my changes impacted the team. I got confused: why would these small changes affect the team? It turned out they could and in a few ways.

I didn’t use my time effectively.

I spent time working on unimportant changes instead of taking on work that mattered. Therefore, someone else had to pick up tasks I could work on. If I did it, we could fix more bugs, implement more features, or ship faster. I also hurt myself – important work is usually a good learning opportunity and can lead to a quicker promotion, but I chose to pass on it.

I stole time from team members.

Code reviews were standard practice on every team I worked on in the past 20 years. Reviewing even small changes requires time. By requesting reviews of unneeded changes, I demanded that my team members spend time on trivialities.

Changing any code can lead to merge conflicts and disrupt other developer’s work. Sometimes, it is unavoidable. But it is not fun when changes no one needs cause conflicts.

I occasionally introduced issues.

A few times, my gold plating resulted in bugs. I missed an edge case in the new code, and somehow, tests didn’t catch it. The bug would break the build or make it to production. Having to justify fixing issues introduced by unnecessary changes is always embarrassing.

Not all refactoring is gold plating.

I am not trying to convince anyone that refactoring, in general, is a waste of time. In most cases, it’s the opposite. Refactoring code often aims to simplify implementing future changes, remove duplication, or make code more understandable. Sometimes, especially when deadlines loom, developers (myself included) make shortcuts or introduce hacks that are a ticking bomb. Removing them is the right thing to do, substantially improving the code quality. These kinds of refactoring are not gold plating. Gold plating is about changes we could live without without anyone noticing them.

Now that you know what gold plating is, whenever you decide to refactor some code, you should ask yourself: “Is it a real improvement, or am I just gold plating?”

Top 5 Unit Test Problems That Haunt Software Developers

Well-written unit tests are one of the most effective tools for ensuring product quality. Unfortunately, not all unit tests are well written, and the ones that are not are often a source of frustration and lost productivity. Here are the most common unit test issues I encountered during my career.

Flaky unit tests

Flaky tests pass most of the time, but not always. They may randomly fail even though no code has changed. The quickest and most common “fix” developers employ is to re-run them. With time, the number of flaky tests grows, and even multiple re-runs are insufficient.

Flaky tests are caused primarily by the following:

shared state
dependency on external systems

A shared state is the number one cause of test flakiness. Static variables could be one example. If one test sets a static variable and another passes only if this variable is set, the second test will fail if the order of execution changes.

Debugging flakiness caused by shared state is usually tricky because sharing state is rarely intentional.

Tests that depend on external systems tend to be flaky because the systems they rely on are outside their control. Any deployments, crashes, or throttling will cause test failures. Network, which is inherently unreliable, is yet another contributor. The best fix is to mock external dependencies.

Multithreaded applications deserve special mention. Race conditions in the product code could make tests for these applications flaky, and finding the root cause is often challenging.

Slow tests

Slow tests are a productivity killer. If running tests for a code change takes more than a few seconds, developers will use it as an excuse to find a distraction.

One of the most common reasons tests are slow is their dependency on external systems: network calls and the time to process the requests initiated by tests add up.

But tests that depend on external systems are also flaky, so slowness and flakiness go hand-in-hand.

Again, mocking external dependencies is the best fix to make tests fast and reliable.

If relying on external systems is intentional (e.g., end-to-end testing), it is worth separating end-to-end tests into a dedicated suite executed separately, for instance, as part of the nightly build.

I was once on a team where running all the tests took more than two hours because most of them communicated with a database. These tests were also flaky, so merging more than one Pull Request a day was virtually impossible.

Bugs in unit tests

Tests are there to ensure the quality of the product, but nothing is there to ensure the quality of tests. As a result, tests may fail to do their job due to bugs. Unfortunately, identifying these bugs is not easy. Paying attention can help. For instance, if all tests continue to pass after changing the product code, it usually indicates either bugs in tests or missing test coverage.

Hard to maintain tests

Tying tests and implementation details closely usually causes numerous test failures after even simple product code changes. Keeping tests focused on functionality instead of on the implementation can significantly reduce the number of unnecessary test failures.

Writing “tests” only to hit the code coverage number

Test code written solely to meet code coverage goals is usually low quality. Assertions in such code are often missing because they don’t contribute to the coverage goal but can cause failures. Test coverage reported by tools can make the manager look good, but this test code is useless as it can’t prevent bugs. What’s worse, the high coverage hides areas that do need attention.

This is my list of the top 5 unit test issues. What’s yours?

Do Unit Tests Find Bugs?

I’ve been writing software for over 20 years and don’t believe unit tests find bugs.

Yet, I wouldn’t want to work in a code base without unit tests.

Why unit tests don’t find bugs?

To understand why unit tests don’t find bugs, we can look at how they are created. Here are the three main ways to handle unit tests:

developers write the tests along with writing the code
Test Driven Development (TDD)
unit tests are considered a waste of time, so they don’t exist

When the same software developer writes unit tests and code simultaneously, the tests tend to reflect closely what the code does. Both tests and code follow the same logic, stemming from the same understanding of the problem. As a result, the tests won’t find major implementation issues. If they find small typos or bugs, it’s usually only by chance.

Test-driven development calls for writing unit tests before implementing product changes. Because no product code exists, the unit tests are expected to fail initially or even not compile. The goal is to write product code to make the tests pass. In TDD, new unit tests are added mostly to drive the implementation of new scenarios. An unsupported scenario could be considered a bug, but it’s far-fetched. As a result, TDD rarely finds existing bugs.

If unit tests don’t exist, they cannot find any bugs.

If unit tests don’t find bugs, why do we write them?

While unit tests are not great at finding bugs, they are extremely effective at preventing new ones. Unit tests pin the program’s behavior. Any change that visibly modifies this behavior should make the tests fail. The developer whose changes caused the failures should examine them and either fix the tests—if the change in the behavior was intentional—or fix the code. Many test failures indicate assumptions that the developer unknowingly broke. Without tests, they would turn into customer-impacting bugs.

Other important advantages of unit tests include:

Documentation – comprehensive unit tests can serve as product specification
More modular and maintainable code – writing unit tests for tightly coupled code is difficult. Unit tests drive writing more modular and loosely coupled code because it is much easier to test.
Automated testing – unit tests are much faster to run and more comprehensive than testing changes manually.

If unit tests don’t find bugs, what does?

There are many ways to find bugs in the code. Integration testing, fuzz testing, and stress testing are just some examples. However, the three below are my favorite because they require little to no additional effort from the developers:

Exploratory testing: Try using the product you’re working on. See what happens if you combine a few features or try less common scenarios.
Code reviews: One weakness of unit tests is that they are implemented with the same perspective as the code. Code reviews offer the ability to look at the change from a different angle, which often leads to discovering issues.
Paying attention: Whenever you code, debug, or troubleshoot an issue, have your eyes open. Many bugs are hiding in plain sight. Carefully reading error messages, logs, or stack traces can lead to identifying serious problems.

The Curious Case of Bugs that Fix Themselves

I can’t count how many times I had this conversation:
  “Good news! The bug is fixed!”
  “Did you fix it?”
  “No.”
  “Did anyone else fix it?”
  “No.”
  “How is it fixed then?”
  “I don’t know. I could reproduce it last week, but I cannot reproduce it anymore, so it’s fixed.”
  “Hmmm, can you dig a bit more to understand why it no longer reproduces?”
  [Two hours later]
  “I have a fix. Can you review my PR?”

Can bugs fix themselves?

I grow extremely skeptical whenever a fellow software developer tries to convince me that an issue they were assigned to fix magically fixed itself. Software defects are a result of incorrect program logic. This logic has to change for the defect to be fixed.

Can bugs disappear? Yes, they can and do disappear. But this doesn’t mean that they are fixed. Unless the root cause has been identified and addressed, the issue exists and will pop up again.

The most common reasons bugs disappear

Seeing an issue disappear might feel like a stroke of luck – no bug, no problem. But if the bug was not fixed, it is there – always lurking, ready to strike. Here are the most common reasons a bug may disappear:

Environment changes

A change in the environment no longer triggers the condition responsible for the bug. For instance, a bug that could be easily reproduced on February 29th is not reproducible on March 1st.

Configuration changes

The code path responsible for the bug may no longer be exercised after reconfiguring the application.

Data changes

Many bugs only manifest for specific data. If this data is removed, the bug disappears until the next time the same data shows up.

Unrelated code changes

Someone modified the code, changing the condition that triggers the bug.

Concurrency (threading) bugs

Concurrency bugs are among the hardest to crack because they can’t be reproduced consistently. Troubleshooting is difficult: even small modifications to the program (e.g., adding additional logging) can make reproducing the issue much harder, which is why concurrency bugs are a great example of Heisenbugs. And the worst part: when the fix lands, there is always the uncertainty of whether it worked because the bug could never be reproduced consistently, to begin with.

The bug was indeed fixed

A developer touching the code fixed the bug. This fix doesn’t have to be intentional – sometimes, refactoring or implementing a feature may result in deleting or fixing the buggy code path.

The bug didn’t disappear

The developer tasked with fixing the bug missed something or didn’t understand the bug in the first place. We’ve all been there. Dismissing a popup without reading what it says or ignoring an error message indicating a problem happens to everyone.

Fixing a bug can be easier than figuring out why it stopped manifesting. But understanding why a bug suddenly disappeared is important. It allows for re-assessing its severity and priority under new circumstances.

Conclusion

If nobody fixed it, it ain’t fixed.

7 Tips To Accelerate Your Code Reviews

One complaint I often hear from software developers is about how long it takes to merge Pull Requests (PR). When I dig deeper, I often find it is all about code reviews. Indeed, code reviews can be a bottleneck, as they require someone to shift the focus from their work to review the PR. The key to faster code reviews is making reviewing PRs as easy as possible. There are several ways to achieve this.

Send clean PRs

You shouldn’t expect to quickly merge a PR with red signals. If your code doesn’t compile, tests fail, or if there are linter warnings you might only hope that someone will notice and ask for a fix. Often this won’t happen. Instead, people will quietly ignore your PR thinking it was sent by mistake.

I wrote a post dedicated to this topic recently

Write a good commit message

The struggle to find someone to review a PR is often a common reason why code reviews take so long. Good commit messages can help with that. A good commit message helps potential reviewers quickly understand if they are the right person to review the change and gives them an idea of how much time and effort the review will need.

Consider these two PR titles as an example: “Fixing a bug” versus “Fixing memory leak caused by retain cycle in X”. Which one do you think will catch more attention? The second title is more likely to attract reviewers because of its specificity. It tells them what the problem is and where in just a few words.

If you would like to learn more, I wrote a detailed post on this very subject.

Ensure good test coverage

“Can you add tests?” might be the first, and the only comment you get on your PR if you haven’t provided sufficient test coverage. It’s easy to understand why people hesitate to review PRs without good test coverage. Adding tests often uncovers issues or gaps in the code. Fixing them might lead to significant changes, that will trigger a complete re-review. Good test coverage not only saves time for the reviewer, who won’t need to go through your changes twice but also for you, as it reduces the number of necessary revisions.

Keep your PRs small

Conducting a thorough review of a huge PR is time-consuming and requires significant effort. Not many team members can commit to such a task. However, most large PRs can be broken down into smaller, logically complete PRs. By doing this, you can distribute the effort to review your changes among more team members, shortening the time needed to merge them.

For a more in-depth discussion on this topic, check out my post.

Address comments quickly

Respond to comments on your PR promptly, to keep the reviewer engaged while the details are still fresh in their mind. Delaying your response can lead to deprioritizing your PR as the reviewer will have to spend time and effort to recall necessary context. Addressing comments doesn’t always mean you need to send a new revision. It is often about providing clarifications or confirming assumptions. However, if the reviewer identifies an issue that requires a fix, you should promptly update your PR.

Ask for reviews

If you did everything possible to make your PR easy to review but it is still not getting traction, you may need to ask for a review directly. If anyone on the team can review your change, consider asking in your team’s chat room. If you would like a specific person to review your PR, contact them individually. However, be cautious not to overdo this. On most teams reviewing code is an expectation, and pinging people for reviews immediately after publishing a PR can be very distracting. In our team, we have a 24-hour rule for non-urgent PRs: we only ask for a review if a PR hasn’t been picked up within a day.

Review your team members’ PRs

If you don’t review your team members’ PRs, don’t expect them to prioritize yours. PRs that are constantly at the end of the queue will inevitably take longer to be reviewed, approved, and merged. That’s why it is important to dedicate a few minutes each day to code reviews. My rule is to review at least as many PRs as I publish. It works wonders – most of the time I can merge my PRs on the same day I submit them. Moreover, if my PRs aren’t getting enough attention, I don’t feel uncomfortable asking to review them.
Here is a post about other benefits of reviewing code.

And when you get your PR reviewed, remember to promptly merge it

What are your tips to speed up code reviews? I’d love to hear them!