Over the last year, I've ended up on four projects that were started prior to my arrival. This isn't uncommon; more often than not throughout any career in development, you'll find that you're spending a lot of time trying to understand someone else's code. It may have been written a few days ago, and it may have been written years ago. The original author may still be on-hand to talk you through it, or they may have moved on. But regardless of the circumstances, you will often find yourself spending time figuring out what a codebase you're looking at is doing, and why.
In a more extreme case recently, I found myself working on a project that had been worked on up until then by two developers, both of whom had been let go (because of budget constraints, not because of the quality of their work, I hasten to add). They had done a great deal of work, but each of them had only worked on one side of the application (one on the front end, one on the back). Regardless of the calibre of these developers, this is a situation that few people would be excited to find themselves in; there were no readme files, no test suites, no code reviews had been carried out at any time, and there was no handover of the project.
After a good few weeks, the project was looking a lot better. Thanks to the archaeology carried out by myself and the fantastic team that I was fortunate enough to be part of, it was documented, there were automated tests, and we had resolved a number of issues that had slipped through the cracks as a result of code reviews not being carried out. But we still shouldn't have been in this position in the first place - and our organisation shouldn't have lost months of (cumulative) development time to this.
And this whole experience made me think a lot more seriously about the code that I write too. And it made me acutely aware of how much we, as developers, end up having to think like a compiler.
Now, that's fine in general - when we're developing new functionality, our brains need to be bent out of shape to consider how the code we're writing will be interpreted - that's part of the job. But when you're looking at a module for the first time, should you have to go through that to understand what that module is doing?
Consider this function (written in Haskell, but the language here isn't what's important), which takes a message and returns a response:
responseFor :: String -> String
responseFor message
| length (dropWhileEnd isSpace message) == 0 = "Fine. Be that way!"
| not (null (filter isAlpha message)) && all isUpper (filter isAlpha message) = "Whoa, chill out!"
| isSuffixOf "?" (dropWhileEnd isSpace message) = "Sure."
| otherwise = "Whatever."
If you're not familiar with Haskell, you probably just skipped straight over all that, right? I don't blame you; what a mess!
But in my experience of picking up other people's code, it feels that a lot of people end up delivering code like this; people will get their code working, and then move on to the next problem. And that's understandable of course. We've all got deadlines, and project managers, and product owners, and stakeholders. We can't make everything absolutely perfect all of the time. But consider how little time it takes to break out the logic of your functions and make them more readable. Literally a couple of extra minutes of your time could save hours of other people's time when they look at your code next - in most cases the time will be saved immediately when your work goes through a code review.
Consider this very quick revision to the above code snippet:
responseFor :: String -> String
responseFor message
| isEmpty message = "Fine. Be that way!"
| isShouting message = "Whoa, chill out!"
| isQuestion message = "Sure."
| otherwise = "Whatever."
Obviously I've omitted the actual logic here - although it is basically the same as in the first example - but by replacing it with useful function names, you don't need to see it any more. If you wanted to know what the responseFor function was doing, you'd be able to tell nice and quickly, unlike in the first example I gave you. You don't have to know that the isShouting function checks to make sure that there are letters in the message, and then checks to see if they're all uppercase, because you're not a compiler. You just need to know that if the message passed in is shouting, the response will be "Whoa, chill out!".
Often people enjoy writing functions that do a lot on a single line, and I'm not against that at all. I like individual functions to be small and elegant, I think that's great. When I'm writing JavaScript, for example, I actively try and make sure that my functions are on a single line so that I can omit the braces from them, and prefer to opt for functions that are composed of other functions. But I'm making a conscious effort these days to ensure that whatever my module's "main" piece of functionality is, it can be understood by reading it like a human, not like a machine. And I've been pleased with the results so far. I reckon that it's worth thinking about the next developer that will look at your code. After all, half of the time it's probably going to be you!
Top comments (14)
Nice article David. This is a great, under represented topic that has bigger consequences than most developers realize as it can lead to failed projects.
Some developers never take time to refactor and simplify their code usually because they don't see a cost benefit, want to implement the next feature with minimal effort, or do not want to deal with the time, communication, and risk of a breaking change. Sometimes a developer has a photographic memory and can remember all the details but the dev behind them doesn't have the benefit of knowing how it was built. The result of this over time is code that they as you indirectly referenced above and the next developers behind them have great difficulty working with. It also hurts their reputation inside the company as someone who generates code that is difficult to work with.
The rule I follow and what I encourage developers working with me is to first get it to work, then clean it up ensuring all parameters are in the configuration file, and all objects, methods, and vars are named to exactly match what they do. Cleaning up later is difficult as the code is no longer fresh in the developers mind and if not done soon afterwards will never get done. Then developers behind them, like yourself, rightfully complain about the code. This is a common issue that has been happening for decades.
A more compelling reason to create easy to understand code is for you as a developer to keep your job, improve your reputation, and ultimately to enjoy the benefits of a long career.
John
Thanks John, I very much agree. And yes, the "it can lead to failed projects" point is incredibly important; especially if you're working for an organisation for whom software delivery isn't their primary function; because, as experienced on the project I describe above, product owners who are not familiar with software development will often not appreciate how big of a task it can be to get up to speed with someone else's project when these considerations haven't been made during its development.
Of course there are often pressures on development teams to get things done as quickly as possible; but we need to appreciate that a small time investment in this area actually allows everyone to get things done more quickly overall. I'm not suggesting this is by any means a new idea, nor a new problem, but sometimes it's worth a reminder. :D
John, I'm sorry. I do not agree that the process should be write nasty code then clean it up. The problem is that the second step probably won't occur - leaving nasty code.
It is far better to insist that clean, understandable code be written in the first place. I bemoan the fact that today we employ entry level programmers to design and implement new code. In the past, we employed new programmer as maintenance programmers who leaned the hard way that poorly written code was to be avoided.
Personally, I advocate coding your comments wherever possible. For example:
should (IMO) be
The only time comments are really appropriate (again, IMO) is for explaining why something's happening which you think is "a bit weird", which in my experience, is down to weird business logic basically 100% of the time. In those situations, yes, do write a comment. But definitely default to trying to code any comments you can!
Really like your point to think about the next developer. Please make sure code can be read in the future, or in other words be maintained by you or others. In order to nail the question how to measure the maintainability of code we share & validate 10 simple guidelines via bettercodehub.com a GitHub integration that is free for open source. We see that these guidelines are easy to learn but difficult to adhere to during sprints. Also any guidelines should not be enforced blindly (resulting in a violation flood), they should be applied to the complete code base and against a large benchmark. Measuring in that way introduces tolerance and provides a Definition of Done. So the developer can think about himself too!
Michiel
Michiel, I think the link is broken. Would be interested to see the guidelines.
Bob, thanks for finding that broken link. The 10 guidelines are in this O'Reilly book Building Maintainable Software and also implemented in BetterCodeHub.com It runs for free with you GitHub handle.
The new version looks beautiful. I wish I had practiced self-documenting code when I was still coding. There's a good checklist here: sitepoint.com/self-documenting-jav...
I would add wrapping any Regex or Xpath in a function with a name in normal language.
Thanks for writing this David. Each and every organisation has this problem and has tackled it with varying degrees of success.
Situitions like this make me pine for programming in BASIC. Anyone could read it and understand what was going on. And that is a very powerful draw.
Thanks again for highlighting this.
@K_Fistman does not exist on Twitter!
I know! Changed my Twitter handle to @_dnimmo but I can't see a way to change it in my settings here. :-(
Hmm we didn't think of that edge case. I'll fix it for you now and make sure it syncs in the future 🙂
Thanks for sorting this out for me. :-)
Great article! In fact, my company just covered this on our latest podcast: dev.to/mousepaw/episode-4-new-char...