// Source control antipatterns

06 September 2017Matt Hilton

Source control is one of those things that's taken for granted in a lot of software teams these days. For many devs now, it's always been there and is just part of the fabric of how development is done. But this wasn't always the case.

Many who've been around the traps a while will remember companies or projects where source code was shared between the team via network drives, USB keys, or perhaps even floppy disks!? In smaller/1-person teams, there wasn't even any sharing: the source of truth is someone's rusty spinning hard drive. Eek.

We've come a long way since then, but incredibly, from time-to-time, I still come across people and teams not using source control, or not using it effectively. For some, it's just "the way things are", rather than a valuable tool for running projects and maintaining software. A key example: in so many teams I've worked with, the codebase is littered with commented-out code which was previously used, but not valid any more. My disbelief is always refreshed whenever I encounter a new instance of this anti-pattern. How did this become a thing? Commented-out code should never be committed to a repository. It clearly demonstrates a lack of appreciation for the power and purpose of a well-used source control system.

How does this even happen?

There's three main things I've seen that cause this problem:

  • Old production code, no longer in use, but kept around "in case we need it" later.
  • Experimental code that's not quite ready, but got committed just to store it somewhere.
  • Code commented out for testing, and accidentally committed without being reverted.

Let's explore some of the reasons these might occur, and how we can prevent them.

Deprecated code

There's a raft of issues with each of these, but the most frustrating is the first one. Let's revisit the purpose of version control: to store a history of all the different versions of our code. If you delete some code but need to retrieve it later, that's the whole purpose of source control: to prevent loss of information.

Solution: Don't check it in. Just delete it. If you need it later, retrieve it from version history. Gah.

Experimental code

I suspect this is a hangover from oldschool centralised source control systems, where files needed exclusive lock to be modified. Perhaps, back then, branching was not as smooth and easy as it is these days. The world has changed though. Branches in decentralised version control systems like Git are cheap, and merging algorithms have come a long way.

Solution: Branching is a thing. Check your experiment in to another branch, don't pollute master with your random experiments. Either make it production ready, or keep it elsewhere.

Acci-debug code

There's really an underlying problem of poor understanding/use of debugging tools causing this one, so should probably be looking at the debugging approaches that caused this in the first place. However, this can cause some of the most insidious bugs because it's very difficult to determine why a piece of code or logic branch was commented out, and it will never be mentioned in the commit logs, because it's an accident.

Solution: As a basic practice, all professional developers should take 2 minutes before checking in code to review what you're committing before you commit it. This practice alone has saved me countless hours of debugging silly bugs I would have accidentally caused by checking in changes I didn't mean to.

When used properly, source control is a powerful thing. It prevents against loss of work, allows us to gain an understanding of the history of a piece of software and why certain changes were made, and modern source control facilitates collaboration, allowing us to work in effective teams without having to worry about file locking or passing USB keys around.

Let's use it properly :)