As a general rule being consistent is better than not being consistent. We create guidelines for how our code looks and what idioms and patterns we use to try and create a codebase that is easy to comprehend so that we don’t then waste brain cycles trying to sift through the trees looking for the wood. But there are times when I find those general rules do not apply, or perhaps I should say, different rules may apply.
Different Domains
In my last two posts about test naming conventions I touched on this very briefly. Tests are a classic case where not all the normal rules for production code apply. Yes, as a general rule, most of them probably still do, but when it comes to naming and code formatting it can be beneficial to do something that helps to convey the behaviour under test. I would break other rules in C# here too such as using public fields, or at least creating mutable properties so that I can use the object initialisation syntax [1]. In C++ I might rely more heavily on the pre-processor to hide some of the technical grunge and allow the reader to focus on the important bits.
A consequence of this is that you can start fighting with your static code analysis tools, e.g. ReSharper, and so being able to suppress certain rules on a per-component basis becomes a necessity if you want to remain free to express yourself in code. For instance with ReSharper I will disable some of the rules about naming conventions and the ones about being explicit where localisation concerns might arise.
Of course what we’re really saying here is not that we’re breaking the rules for the codebase per se, but that there are really two different dominant domains within the codebase that serve different purposes - production code and test code. As such we’re not really breaking the production code rules but defining a different set of rules for the test code. But it’s important to realise that this distinction occurs and that it’s a valid approach to take. Even within the production codebase there may be some smaller domains that are better served by loosening the rules, such as configuration, but this may be driven in part by an inability to configure a tool to meet your desires [2].
Refactoring
For production code the decision to do something different can often be much harder to justify. The desire is often to be consistent even if an earlier decision seems like a bad choice now, and the team decides that in future they believe a better approach should be adopted. During the period of change the codebase lives in a state of flux with two opposing sides - one demanding consistency and the other promoting a better life. Whilst the old timers will know about these changes in direction the new joiners wont and so they will likely follow whatever they see first, which may give the wrong impression. Unless you are pairing or have code reviews the opportunity to guide them in the new direction might be missed and the old ways will persist even longer.
One example of this was the change in naming convention in a C# project I worked on. The project was started by a bunch of ex-C++ programmers and so we adopted the all-uppercase convention for constants. Quite a bit of code was written before someone started using ReSharper which complained about the convention and so we investigated why ReSharper was suggesting a different style. We realised at that point that our style was not idiomatic and, more importantly, it actually made sense not to do it. So we decided that going forward we would drop some of our old inherited C++ habits and follow the more traditional C# conventions.
This might sound as if I’m contradicting my advice in “Don’t Let Your Tools Pwn You”, but really I’m advocating how we used it as a tool for learning. The problem was that only one of us had a license for the tool and there was now quite a bit of code that violated the conventions. What to do?
We had to decide how we wanted to move forward and the only way forward appeared to involve us going against our principles in one way or another. Trying to go back and fix all the code up at once seemed like a criminal waste of time as it wasn’t dangerous; just not exactly what we’d like to see now. If we add new constants where there are already ones in the old style, do we follow the existing code style or adopt the new one thereby mixing different styles. If we fix up the style of the old constants whilst adding the new ones we create a commit that includes functional and non-functional changes which we tried to avoid. If we fix up just the old subset where the new constant goes in a separate check-in we then create other bits of code of differing styles.
While it might seem like it took longer to explain this problem than to correct it, especially given that this particular example can be largely automated, it does illustrate the tensions that can come into play. Sometimes the problem has to get worse before it can get better. For example we tried using a 3rd party mocking framework in a number of different ways as we couldn’t all reach a general consensus about a style we liked. Eventually we discovered a way of using it that supported more like what we were after, but there was no automated way of going back and re-writing all those existing tests in the preferred way.
Guidelines, Not Rules
I find the following quote from Pirates of the Caribbean nicely sums up the attitude we should take towards any “rules” we might establish to try and maintain a consistent codebase and development process:
Elizabeth: Wait! You have to take me to shore. According to the Code of the Order of the Brethren...
Barbossa: First, your return to shore was not part of our negotiations nor our agreement so I must do nothing. And secondly, you must be a pirate for the pirate's code to apply and you're not. And thirdly, the code is more what you'd call "guidelines" than actual rules. Welcome aboard the Black Pearl, Miss Turner.
Many of these latter problems probably stem from an immaturity with the toolchain, but even in an established language like C++ things change over time. The recent C++ 11 and C++ 14 standards both introduced sweeping changes that have caused developers to question past practices and to unlearn what they learned so long ago. We need to remember that our processes are never cast in stone but are fungible and need constant refinement as the programming landscape around us changes.
[1] Modern C# supports named arguments and so this is less of a concern.
[2] Some C# serializers make it hard to work with immutable types by default because of the nature of the reflection based approach they use.
No comments:
Post a Comment