As a team leader who also writes code I have to worry about code several times over.
- The coding standards we adhere to – they must be disciplined but not overbearing or pernickety
- Every line of code the team writes – the objective is a decent product made a different way. While peer review is the way to ensure everyone takes collective pride in the work, the tech lead doesn’t get to stop worrying.
- Every line of code I write – what kind of a person doesn’t act in the way they demand in others?
Recently, I’ve been noticing issues in the way we’ve been working. It only takes a few minor cases of letting our standards or techniques slip for our efforts to become counter productive. As team lead, I could point the fingers at the individuals who happened to write lines of code that I came to worry about. I don’t need to. As a contributor to that code, I can find recent examples where, for no reasons other than a combination of bad luck and time pressure, I dropped some balls. I then discovered the effect these minor slips had, and I’d like to confess to them.
Making mistakes is no big issue; learning from them is a great opportunity. I hope others will find this useful.
Name it after the implementation
We had the need for a hashing key which could be easily predicted. We settled on a date with two random digits after it. This would hash well, but you could, for any given date, predict the hash key to within 100, which is easily searchable. So we did a lot of talking about random.
Two bad things happened. This was a consequence of the noun random getting stuck into our discussion, when we were really making a predictable hash key.
Firstly, the code got filled with the word random, which others were asking about – why is random? How will that work?
Secondly, we made the code depend on a random number generator. Given that we were trying to make a hashing function which was going to be used for persistence, the ad-hoc random number generator, coupled with whatever Java happens to do with hashCode implementations, could best be described as something which works that way for now.
All of this was a consequence of thinking random first, rather than seeing that statistically those two digits would be random, but we were trying to make a stable hash key. In the end, I switched it to be a two digit sample from an MD5 hash. This I tested for statistical variability and it was fine.
What’s the best way to represent a day code for computing? We ended up with YYYYMMDD as an 8 digit number. That’s definitely a day. Isn’t it? My mistake was to try to process this as a number. Given all the days between 20161101 and 20161202, you can just increment the day number, right?
20161130 leads to 20161131 (?) and then 20161142 and 20161199 – these are valid numbers but they’re not valid days.
Where in a later part of this article I’m going to argue the opposite, it was clear at this point, that the unnecessarily human-friendly more complex-to-constrain number would have, if kept, led to more code around it to manipulate it. Totally misleading. I zapped it and replaced it with epoch day.
What unit tests?
I promise you. I practice test-first development. I do it a lot. I teach it. I believe in it. I’ve used it to help me out of situations where I couldn’t get something to work, and the high discipline incremental nature of it has given me revelations.
I jumped into a system wide refactor, algorithm change, and rework of some core code. Did I run the tests? Apparently not. I didn’t feel I had time, or wanted to make time, to run the tests – surely what I was doing was just going to work, right? I had all the answers in my head.
In fairness to me. I didn’t get too far out… but then I actually ran the tests. That’s when I discovered the problem below about typing. My code worked in seconds, other code worked in milliseconds. They don’t match. It didn’t work. It would have worked.
At this point, I felt very privileged to work with a team who have been trying to adopt the principle that I forgot for a couple of rabid hours. One of them had written some brilliant step by step unit tests for each feature. I got them to work one after the other by fixing the code and they guided me to my destination perfectly. Just think how much use they would have been 4 hours sooner!
Strong typing replaced by…
I wanted to represent a search time range. I had an object which happened to store the time internally as a long. This is fine. It serialises. Long is a common way to store time. It’s milliseconds, right? Or seconds? Epoch seconds? In UTC? Or epoch milliseconds. Surely it’s encapsulated…?
A friend of mine complained that we weren’t always using strong typing, we were something using String typing – where the info is just sitting there in the string in the right form if you know how to interpret it. What of abstraction and encapsulation? In the above situation, we didn’t have strong types, we had Long types.
Why was this an issue? Just choose milliseconds or seconds and it would work again? Two other things:
- Occasional use of Joda time to help with the object
- A module which manipulated the inputs to put into a time range object because it knew how to do things in milliseconds
I felt shocked. Here we are with Java 8 and we’re trying to operate on raw numbers from outside a class which is all about time in an environment where there’s the finest time library in the world baked into the language!
This was a poor design decision of mine taken literally by the team around me. I refactored my way out of it by introducing an external interface entirely composed of Java 8 time classes. The internals of the class remained as Long because that serialises in a compact way. The outside world was not allowed to push Long values in any more and a number of helper methods were absorbed into the time range class itself using the ask, don’t tell principle.
Users of this weakly typed object are left now with no doubt how to interpret what it means.
The above weak typing is ameliorated if there are some decent examples of real world usage in the unit tests. If, however, you’re just dealing with simple values, you may be tempted to write tests in terms of unrealistic numbers like 1234, or 0. These can test for things like equality and comparison, but they also give no indication of what sorts of real-world usage might happen and whether the code would work predictably with real world numbers.
For example, if I tested my YYYYMMDD day calculator purely in terms of numbers like 11111111, I would not notice that there appear to be 70 days between the end of November and early December.
In many cases, there’s no such thing as a good or bad input to an algorithm (I do a lot of property bag tests using the String Jim). However, if you have an abstraction and you don’t test it with real world inputs, you’re missing the tests document the code opportunity and may be sitting on it only works in theory problems.
There are two ways to interpret it when someone makes a lot of code and passes it to you. On the one hand, they’ve just delivered something of value that you should be able to use. On the other hand, it can feel like a hit and run. The code may or may not be fit for purpose. It may or may not have nuances that you can understand right away. It may be a boost or a few hours of head scratching waiting to happen.
On the whole, parachuting your code onto someone else’s to do list is definitely something to do with caution. I know what it’s like to see people really get a productivity boost from having an answer handed to them. I know what it’s like for some unimportant detail in that to steal time. I know how it feels receiving a batch of someone else’s incomplete work. In short, it’s a thing I’d like to see less of.
Documentation is for Wimps
My firmly held view is that documentation is not to be an input to development, but more of an output. I don’t think that technical documentation is innately valuable, especially where it can get out of date with the code. However, you need to leave something for the next person who needs to be able to use what you’ve made. That next person may well be you!
- JavaDoc – public APIs to have their semantics described
- Code review – just accounting for your changes to someone else in a discussion, especially where you comment on your own code to explain why you did it that way, it can really help you see your work from the outside and make some last minute refinements or simplifications
- High level diagrams – if you can’t draw a diagram of your system on the back of a beermat you don’t understand it – if you can create high level diagrams as part of development then that is very helpful. They seldom get that far out of date. Extra points if you can get the diagrams to be generated automatically as an output of your work.
- User guide – if you make a feature but it relies on the developer knowing exactly where to find it, and the exact semantics of using it.. well, you’ve failed. There should be a human-friendly interface, some of that may be a start-up script or a how to document, or just a well documented public API entry point.
I’ve not been strict enough regarding documentation. I really don’t want to force people into being authors… but the definition of done has documentation mentioned and I’ve been less focused on where the minimum threshold actually is. One of my mistakes came about because I had no documentation to guide me.
In the cut and thrust of development, it’s no surprise that sometimes one’s standards slip. The aim should be to commit to work which can be achieved at a sustainable pace. That’s no guarantee that there won’t be blips. The simple provable truth, though, is that dropping discipline when under pressure more often results in a spiral of rework as the poorer techniques appear to be less effort, but result in more confusion and rework.
I’ve reminded myself of a few useful points here. I hope others find this useful too.