Deliberate coding

Matthias Noback

I wanted to share an important lesson I learned from my colleague Ramon de la Fuente. I was explaining to him how I made a big effort to preserve some existing behavior, when he said something along the lines of: the people who wrote this code, may or may not have known what they were doing. So don’t worry too much about preserving old stuff.

These wise words eventually connected to other things I’ve learned about programming, and I wanted to combine them under the umbrella of a blog post titled “Deliberate coding”. Doing something “deliberately” means to do it consciously and intentionally. It turns out that not everyone writes code deliberately, and at the very least, not everyone does it all the time.

Decisions

It turns out that, when I look at a piece of legacy code, I’m making a few assumptions:

  • Every line of code is there for a reason, it has been explicitly decided that it should be this line of code. E.g. it embodies a piece of domain knowledge the developer had gained, it represents an exact requirement as provided by the product owner, it’s a fix for a bug in some exotic situation, etc.
  • Every aspect of the behavior emerging from the code is relied upon by at least one user. Whenever I change something, it should be refactoring in the true sense of the word: changing the structure of the code, without changing its behavior, so the user isn’t put off the track.

As I write this out, these are clearly not such good assumptions. Whether or not we can trust the code to be meaningful, important, to have been deliberately written down - that depends on a lot of things:

  • How were the developers “managed”?
  • Did they actually speak with domain experts?
  • Did they use some kind of test-first approach? Did they think about testability?
  • How well did they know the programming language itself?
  • Did they have a step-debugger installed?
  • And so on…

This could be a very long list actually.

Documenting the intention behind the code

I remembered something Cyrille Martraire wrote in his exhaustive treatise about the topic of “Living Documentation”:

Building software is about continuous decision-making. Big decisions usually get a lot of attention, with dedicated meetings and several written documents, while other “less important” decisions are somewhat neglected. The problem is that many of these decisions end up being arbitrary rather than well-thought about, and their accumulated effect (perhaps even a compounding effect) is what will make the source code hostile to work with.

Maybe over time, the decisions that the programmers made were not always made consciously. Maybe, they were not based on domain knowledge, or actual requirements. Possibly, a line of code is just there:

  • Because adding that line seemed to make it work,
  • because the same line was also there in a similar situation,
  • because a manager told them to add it,
  • because it was a copy-paste error, or an accidental Duplicate line keystroke (remember goto fail;?)
  • because it was a leftover of the old situation, or maybe
  • because a manager didn’t tell them to remove it.

This is why the intention behind the code needs to be documented. The code itself could be its documentation, and so could the history of the code, as it is recorded by your version control software. But tests/specifications are the best things to happen to code. Without tests, lines can be added, modified or removed and the developer who later reads the code, won’t know if the code (and all of the code) is there for a good reason.

Writing code consciously

So I learned not to trust every line of code I encounter, and not to preserve all of its little (weird) behaviors. But, how to make sure that people who come after me can trust my code? Or, generalized: what can any developer do to write code consciously?

Well, the answer is a big one. Follow clean code rules, adopt a test-first approach, apply Domain-Driven Design, with related practices like Living Documentation. Anyway, it’ll be impossible to cover all of these rules in this article. I thought it would be interesting though to briefly discuss some programming practices I noticed over the past few years, that stand in the way of deliberate coding.

Examples of non-deliberate practices

If you’re looking for things that stand in the way of deliberate coding, keep an eye on the things you do without thinking. Find the advice you’re following, without making a conscious trade-off. Look for code that you didn’t write yourself (that had to be copied, or auto-generated).

We generate code, boilerplate, doc blocks

I find that I’m often looking for ways to let my IDE do things for me. Things that I usually don’t (have to) write myself anymore, because my IDE has shortcuts for them, or offers me to add them, are:

  • Constructors
  • Getters
  • Doc blocks
  • Closing HTML tags
  • Test classes

If you use tools to generate controllers, console commands, or even entities, you’re producing even more code that was not written by yourself. This usually means you’ll end up with lines of code or comments in your files that are not there on purpose. Sometimes you don’t even know why they’re there or what they achieve, and sometimes you just forget to clean them up.

My advice: consider the trade-off. These convenient shortcuts and code generation tools will save you a bit of typing, but may bring code into your project that you don’t really “own” (i.e. need, understand, maintain, etc.). They are okay to use, if you later revise the generated or copied code.

Also, keep an eye on how your IDE evolves. At some point I noticed that auto-generated doc blocks also contained exceptions that were thrown in the method. It should be a very conscious decision by you and your team, whether or not you want @throws annotations in your doc blocks. At the time of writing, my opinion is that for the most part it doesn’t make sense to add them. But in some cases, it does, e.g. in interfaces. Explicitly mentioning which exceptions can be expected by the client will make them part of the interface’s published API.

We auto-fix IDE inspections, etc.

I often find myself “fixing” code because the IDE tells me to. My IDE and the inspection plugins I’ve got installed warn me for all kinds of issues that I would’ve never been aware of if I’d still been writing code in Notepad (or Dreamweaver, like I did in 2004). I think this is quite a dangerous thing. When my IDE started adding @throws annotations to my doc blocks, it also started warning me about missing @throws annotations.

The idea may have come from an IDE developer with a Java background (nothing wrong with that of course!) - and they must have missed it in PHP, or at least found that the IDE should promote the Java practice of explicitly mentioning exceptions. It turns out there are good reasons for this practice. But we should not let an IDE take over existing programming principles just like that.

Of course, it’s really great that IDEs help you write better code from the start. And I love most of what they do. But I also notice how following its suggestions, trying to get rid of smaller or bigger warnings, leads to code that you no longer deliberately write. So my advise on this: if your IDE suggests a change, think about it and discuss with your team whether or not you want to follow this particular type of suggestion.

A nice example is when I write:

mkdir(__DIR__ . '/' . uniqid('', true));

And PhpStorm (in fact, the PhpInspections plugin) tells me to change it to:

if (!mkdir(__DIR__ . '/' . uniqid('', true)) && !is_dir(__DIR__ . '/' . uniqid('', true))) {
    throw new \RuntimeException(sprintf('Directory "%s" was not created', __DIR__ . '/' . uniqid('', true)));
}

If I press Alt+Enter, it happens automatically. The change should help prevent race conditions. If this is your bug, it’ll be an interesting one to fix. But the inspection plugin doesn’t know about the context of this line of code. Maybe I’m writing the mkdir() statement in a script of which I’ll never run more than one instance, so this concurrency issue may never occur. But more particular: in this case, I’m using a unique directory name, so I better hope that no other script is trying to create the same directory at the same time. Also, take a look at the replaced code. It won’t work at all, since the directory name will be different every time uniqid() gets called.

What I’m saying is: think before letting your IDE write code for you.

We collapse code

One thing that annoys me is automatic collapsing of code. It basically says: these lines are not interesting, so I don’t want to see them. Still, these lines are part of the repository, you preserve these lines, you may need to maintain these lines (like: read them, update them when necessary, clean them up, etc.). If you don’t see these lines, you won’t maintain them. Common examples of collapsed code are doc blocks and import/use statements.

Besides being a thing you have to maintain, you should actually keep an eye on your import statements, since they will be telling you a lot, like:

  • Is this code coupled to the framework?
  • Is this code adhering to the Dependency rule?
  • Does this code know about too many domain concepts?

So definitely don’t collapse code (or only temporarily, to de-clutter the view on a piece of code you’re working on).

Conclusion

In the bigger picture, my suggestions above about using your IDE well (and when to ignore it) may be a bit unimportant. In the end, it’s about making an effort to code deliberately in every way possible. But there’s an even higher level of coding deliberately that I think is worth considering: the ethical dimension of writing code.

Every line of code represents an ethical and moral decision.

Grady Booch

I just don’t have a lot of advice on this topic yet… Let me know if you have good pointers about the topic!