Whatever happened to code reviews?
I have read an argument that a programming language can enforce good programming style by restricting programmers to constructions and mechanisms that are deemed worthy, or perhaps are restricted to a small set so that there is an enforced consistency in a code base. (I am not talking about using compilers to detect bugs or memory management being used to prevent bugs, I am talking about programming languages that do not permit certain kinds of abstractions outright).
The premise of the argument is that if programmers are constrained to a single, universal “good style,” good programs will result.
responsibility and authority cannot be decoupled from each otherIn many other forms of management, people have attempted to automate the manager out of a job. There is an idealistic notion that if we write just the right series of instructions for people, if we build the perfect instruction manual, if we detail every possible case and script every possible behaviour, we can build a team that always does the right thing. We see that in highly industrialized occupations like preparing fast food: there is a detailed, step-by-step instruction for everything, and employees simply follow the rules.
Many things have been written about whether that works or doesn’t work for software developers. I am now talking about why that doesn’t work for teams, for team leaders, and for development managers. Every thing you put in place in an attempt to enforce good behaviour takes something away from the team’s responsibility to manage the process using judgement.
a false sense of securityWe joke about a compiler generating a false sense of security: it compiles, so it must work. But that is
exactly the mind set of choosing a language on the basis of it actively preventing poor style: we are choosing it because it will relieve us of the work of manually reviewing code and analysing whether it is of good style or not. It will relieve us of the work of setting a good example and evangelizing good code. It will relieve us of the work of teaching, of leading, or managing.
In the fast food example, we know that the “managers” in the restaurants are not true managers. They have little or no authority to deviate from the rules. And for that reason, they have little or no accountability for what results: you can fire them for failing to follow the rules, but if they follow the rules and bad things happen, it is the rules that are at fault, not the manager.
And so it is with enforcing “good style” with a programming language. If someone writes code in that language, they are absolved from all responsibility for its style. And so is their manager: the programmer’s code compiled, it is demonstrably free of bad style, what can we demand of their manager other than to see to it that they use the language?
Software development is a young profession. We don’t know an awful lot about what works and what doesn’t work. but I’ll take a stand here: We humans have an awful lot of experience with the relationship between authority and responsibility. And what we have learned is that if we remove an individual’s authority to choose, we absolve them of responsibility for the result, and ultimately performance suffers greatly even if their skills are objectively high.
automatons on the style councilDoes this mean that automation is bad? Why do we use high level languages and compilers? Why do we use libraries and frameworks? Don’t those tools also relieve us of responsibility?
Code Complete is one of the greatest books ever written on the subject of shipping software. There is a reason it is subtitled “A Practical Handbook of Software Construction:” Every page is loaded with insights and practices that can help you be a better developer, help your team be a better team, help your manager be a better manager, and help your organization ship software. The wisdom about code style, quality, and review will pay for the book many times over.
Well yes, of course they do.
They relive us of responsibility for things we have deemed not to matter. For the vast majority of lines of code, the exact translation to CPU op codes is unimportant, as long as it works. Nobody cares to look under the hood. Compilation is the process of automating the accidental complexity of our CPU.
If you choose to use a programming language with lazy evaluation, you have decided that evaluation order is an accidental complexity that no longer matters to you, and you want the language to sweep it out of sight. What we choose to automate speaks to what we consider irrelevant. As long as it works, we don’t care how it’s done.
So when we automate style, we are deciding that style dosn’t matter. As long as it passes the compiler, it is good style.
1Automating good style ultimately weakens the notion of individual and shared code ownership, it emphasizes the notion that if it passes the gate, it must be good. That, in turn, creates the illusion that no further inspection is required. If it functions as we require, and it is written in the language that enforces good style, why do we need to review it?
We can talk about why role of a programming language should be to
enable good style, and have a terrific debate about what that means and which languages do a good job with it. But it is more important to keep our eye on the ball. The most important factor for the success of software projects is still the people. And what makes people produce good software? Taking responsibility for their work. Ownership and accountability.
Ownership and accountability is weakened when you attempt to constrain the programmer and it is enhanced when you provide a programmer with freedom and follow up with review. Thus, my question:
Whatever happened to code reviews?
- Python’s significant whitespace is a good example of this. By enforcing a standard indentation, Python says that indentation isn’t worth thinking about: everybody does it one way, and that’s a benefit for readability. Compare and contrast this to a hypothetical language Imperitivity. Imperitivity does not have anonymous functions or closures as first-class objects, and many Imperitivity programmers argue that they should not be added to the language because that would be bad style, or it would promote inconsistent style with some people using closures and some not.
Would we seriously equate the two positions? I think not: whitespace really is pretty trivial: the fact that we can write a script or IDE plug-in to “fix” indentation to match a style guide is evidence that it should not take up valuable team and management attention.
However, the question of whether a code base should include closures, should not include closures, or ought to enforce a consistent approach to closures is important. Using a language to settle the question is an abrogation of responsibility: the matter should be decide by the team or technical leadership and enforced through review, mentoring, training, example code, and so forth.
The salient difference is that while consistent indentation is useful, indentation itself is unimportant in comparison to other technical choices the team must make. Important choices, such as the appropriate paradigms or idioms to use, ought to be decided by the team and enforced through review.