Who the Heck Wrote This? 3 Ways to Deal With Bad Code

Who the Heck Wrote This? 3 Ways to Deal With Bad Code thumbnail

We’ve all been there. After hours or even days of tirelessly trying to narrow in on the cause of some small bug, you finally close in on a particular section. You know the problem is in there somewhere, but the code is spaghetti—an impossible-to-read, jumbled mess of logic that does who knows what and has zero comments. "Who the h*&% wrote this?" you mutter under your breath. As you scan the problematic code for the umpteenth time, you wonder how this even made it into the code base in the first place.

The way you respond to this dilemma has to do with your development as a coder. I’ve broken that process into three phases, each based on a different response to the question "Who the h#%$ wrote this?".

Phase 1: The No Risk Fix

goto
Source: xkcd

Usual Suspects

Most likely a young developer who is new to the code base. Also occurs in seasoned veterans who are looking to put in a quick fix and move on.

Rationalization

"This was probably written for a good reason, I shouldn’t change it too much."

"I don’t want to break anything else, I shouldn’t change it too much."

"Well, Jim is a really smart guy and he wrote this; he probably did this for a good reason."

Coding Approach

Change as little as possible by adding a special case to only change anything if a very specific code path is followed.

How It Looks

Lucid Software was my first programming job and the first time I was working on an application where bugs had a real impact. We had real users using the software who would actually see the bugs. I was REALLY nervous and did not want to break anything.

One of my early assignments was to fix a bug in some of our shared code between Lucidchart and Lucidpress. It took me longer than it probably should have, but I eventually figured out where the problem was occurring. The code was complex, but what it needed to do was relatively simple. I eventually found a working solution. As I was getting ready to check it in, I thought "You know, this bug was only reported in Lucidpress; I don’t want to change anything and accidentally break something in Lucidchart." I quickly patched my fix by wrapping it as follows:

if (lucid.app.isPress) {
    // Do the fix
}

A year later, a developer from one of the Lucidchart teams came down to ask me a question about some of the code I had written. Sure enough, it was this piece of code, and the bug had now been reported in Lucidchart. Man, was it embarrassing when I had to tell him there was no good reason for the special casing, it was just because I didn’t want to break anything else. Instead of risking breaking something, I just let the bug live on for an additional year.

The Takeaway

While adding a special case may be safer and faster, it generally does not solve the real problem. It leaves other issues behind and can make the code unnecessarily complex.

Transition 1.5

The transition from Phase 1 to Phase 2 is about accountability. A willingness to be accountable for the changes you make to the code is essential. That said, it is understandable that a new developer in a code base wouldn’t have the confidence to refactor and make large changes. As a developer increases in skill and familiarity, they move onto Phase 2.

Phase 2: Clarifying Comments

Note to future self
Source: xkcd

Usual Suspects

Developers in Phase 2 are typically more familiar with the code base they are working on and are starting to understand dependencies and the repercussions their changes will have. They have seen enough of the code base to want it to be improved but are still not confident enough to rewrite major pieces.

Rationalization

"I really should change this, but I don’t want to mess anything up too badly."

"I wonder if Jim meant to do it this way."

Coding Approach

Add comments, rename variables, and change indentation or styling to make the code more readable and understandable.

How It Looks

About a year into my time at Lucid, I took on a story dealing with our backend infrastructure in PHP. The code I was working in took a JSON response from our documents service, parsed and determined the correct documents to show, and then sent a consolidated list to the front end that would be displayed to the user. The PHP code was a wreck, an artifact from before we started porting to Scala. It was still the original document controller from when the entire backend was one monolithic PHP app. I had never written any PHP so it took me three full days of just reading and rereading code to understand what was happening.

Eventually, I figured out where the problem was (it was literally just a one-line fix). Luckily, my code reviewer had more insight than me and asked me to add some comments and refactor the code to make it more understandable. I had spent 3 days trying to understand the code, and the next person who had to fix an issue there would have to take just as long.

I took one extra day and reworked my pull request to include some in-depth comments, documentation on methods, and variable names that were more reasonable than someDocData, firstStep (yes, that was a method name) and temp. Overall, the code was more readable and usable for any future developers who would come behind me.

The Takeaway

Developers will have to follow in your footsteps. If it takes you a while to learn and understand something, try to leave the code in a better state so the next developer won’t have to take so long.

Transition 2.5

The transition from Phase 2 to Phase 3 marks an important turning point in a developer’s career. It turns him or her from someone who is able to fix bugs and add new code into someone who can remix and improve on old ideas,making the code more robust and understandable for everyone. This key to this transition is confidence. It occurs when the developer grows confident not only in their understanding of the code but also in their ability to improve upon it.

Phase 3: Delete and Rewrite

Code quality
Source: xkcd

Usual Suspects

This is most likely to be a seasoned developer within the code base. They know the ins and outs of the product and code and understand how things should work. They have seen bugs come and go and acknowledge that they have probably introduced more bugs than they have fixed. They have worked all over the code base and, as a result, they understand that code written before is not infallible. In fact, they believe it’s probably wrong.

Rationalization

"Whoever wrote this is an idiot."

"I really hope I didn’t write this."

"What was I thinking when I wrote this?"

Coding Approach

Scrap what was written and rewrite from scratch.

How It Looks

My first run in with a Phase 3 developer was sitting down with our CTO, Ben Dilts, and asking for help with a bug I had been looking at for roughly a week. It was deep in our text rendering code, some of the most complex code in our system. At this point, I had given up and knew I needed help.

I showed him where I thought the issue was and explained that I was struggling to understand what the method was doing. He turned to me and said "Well, we know what the method is supposed to do, let’s just make it do that." He then proceeded to delete the entire section and started rewriting from scratch. Our new solution turned out to be much simpler and roughly 1 / 4 of the size of the original solution. It benefitted from some new methods that had been written in the last three years (since the code in question was last revised) as well as lessons learned and experience gained.

What stood out to me was that the code we rewrote had been written by Ben originally. He did not take offense that his old code had bugs in it, or had become outdated and less efficient that what we had now. He was quick to accept it and just jumped in to fix the issue in the best way he knew how. Because of this development work, and the development work of many of our seasoned veterans, our code base is constantly improving and becoming more clean and understandable for anyone who may read it.

Conclusion

As the old saying goes, "If it walks like a duck and quacks like a duck, it’s probably a duck." The same is true for bad code. If it looks like bad code and smells like bad code, it’s probably bad code. The infallible developer does not exist. The almighty developers that came before may have made a mistake, and you will too. It is how coding works. As we build iteratively on the solutions that have come before, we will improve the code base. So when you ask the question "Who the heck wrote this?" it should be inevitably followed by "What can I do to make it better?"

Good code
Source: xkcd

8 Comments

  1. Itay ElbirtMay 19, 2016 at 6:43 am

    Fun read.
    I think its important to note that a common beginner’s mistake is to start from ‘phase 3’ due to ego. Something like the opposite of “The Humble Programmer”.

  2. James kelseyMay 19, 2016 at 8:55 pm

    Good read. I went through these refactoring my own code on a personal project. Including asking “who wrote this?” im a different programmer after 6 months.

  3. Florian Seidl-SchulzMay 21, 2016 at 4:21 am

    Rewriting old code as the final- best solution? Agent Spoelsky – take him away.
    http://www.joelonsoftware.com/articles/fog0000000069.html

    Did you refactor it or actually write it new from scratch? One is Information transfer- the other is lossy. Also how big is the rewritten part now?

  4. A RayboldMay 22, 2016 at 7:57 am

    Your mistake – if it was that – in the first case was not in limiting the scope of the fix, but in not asking what the scope of the problem was.

  5. Very funny and also real. Thanks for this article. Valuable information and experience sharing that I appreciate.

  6. Dmitry PashkevichMay 27, 2016 at 11:38 am

    A Raybold, I see where you’re coming from, but it’s not uncommon to come across unexpected bugs when working on something else. What do you do then?

  7. Sometimes a Delete and Rewrite is appropriate on a small scale. What Joel’s article was mainly concerned with was a wholesale rewrite of an entire large product. If you need to delete and rewrite a single function from scratch, then that might be appropriate. It is all about the level of risk you are willing to take.

  8. Found this after reading some code and knew someone had to have written about these problems. Some of these problems deal with simple pro’s and con’s for the individual in a given environment. Writing quick spaghetti code in the right environment can produce multiple positive situations for the individual directly involved. First, if they finish faster, it looks more impressive to non-programmer bosses. Secondly, if later on, another programmer has to deal with the code and appears slower because of the mess, there is a chance a boss may see that as a sign that the latter programmer is inferior to the first programmer. You see a lot of this in programmer code in which the programmer is not directly attached to a team and is allowed to run wild. Some are good about following good practice, others just pump things out as fast as they can with no thought on what comes later. I’m dealing with an application that allows users to change the primary key on database records. lol.

Your email address will not be published.