PJSen Blog

There is a rule called The Boy Scout Rule. In essence, it says that whenever you attempt to modify code, you should also try to improve something in the existing code so that you leave it a little bit better than it was before. By following the rule we can gradually and seamlessly get rid of technical debt and prevent deterioration in software systems.

The rule can be addressed at organizational level in an interesting way. I have come across an idea of a project manager who was responsible for multiple teams dealing with significant amounts of legacy code. They introduced some kind of a gamification to the development process. The teams were supposed to register as many improvements in code as they could and a team who got the biggest number won the game. The prize was some extra budget to spend on team party. Such idea may not be applicable in all organizations, but it clearly shows how to methodically approach the problem of technical debt at management level.

Although I do not immediately recommend the idea of gamification, but I certainly recommend creating some static (not assigned to any sprint) ticket for all the improvements and encourage developers to make even smallest refactor commits under such ticket during they normal development tasks. Below I would like to show some basic indicators that in my opinion qualify for being improved as soon as they are discovered.

1. Improper naming causing an API ambiguity

   I see a few problems here. When I first saw client code calling GetValue I thought it returns some custom, domain specific type. I needed to search for a method returning string and I skipped GetValue, because it did not look like it returns a string. It was only later that I realized it actually does return a string. If it returns a string, it should be named appropriately.

   More general observation here is that we have 3 ways of converting the type into a string. In my particular case I had 10 usages of GetValue, 45 usages of the operator and 0 usages of ToString in the codebase. When talking to the maintainers, I was told there was a convention not to use the ToString method. That situation clearly shows some adjustments are needed both at the level of code quality and at the development process level. I have nothing against operator overloading, however it is not very frequently used in business code. The code readability is a top priority in such case and being as explicit as possible is in fact beneficial from the perspective of the long term maintenance.

   The unused method should obviously be removed, and the one returning a string should be named ToString. I would keep the overloaded operators, because why not, but I still am a little bit hesitant about using them in new code. It is cool language feature when you write code, but it appears not so cool when you have to read it. Even here, I would consider sacrificing the code aesthetics of the operator in favor on simplistic ToString.

2. Misused pattern causing an API ambiguity

   This one is very similar to the previous one, as it boils down to the fact, that we can instantiate an object in two ways. When I was introducing some modifications to the code, at first I was forced to answer the question: should I use the constructor or the Create method. Of course, it turns out, indeed there is a slight difference, because Create returns a result object, which is a common way to model logic in a kind of functional way. But still, at the level of API surface we do not see the difference clearly enough.

   The gist of that case is, there is a pattern in tactical (I mean, at the level of the actual source code) Domain Driven Design to use private constructors and provide static factory methods. The purpose of that is primarily to prevent default construction of an object that would render it in a default state that is not meaningful from the business point of view. Also, factory methods can have more expressive names to indicate some specific extra tasks they do.

   The constructor should be made private and the factory method can be named CreateAsResult, if the wrapper type is prevalent in the code base.

The ideas behind such improvements can actually be very simple. Some of them have to do with trivial, but extremely relevant conclusions about engineering a software. For example:

• any piece of code that slows down a programmer maintaining the code can potentially be considered not good enough
• the code is written once, but read multiple times and thus, when writing a code, we should optimize for the easiness of reading it

The vital part of that mindset of clearly expressing intention is proper naming. I highly recommend watching excellent presentation CppCon 2019: Kate Gregory “Naming is Hard: Let’s Do Better”. It helps develop a proper way of thinking when writing a code.