Simpler rules, better code
There’s a lot of talk today of how learning to write good code requires writing millions of lines of bad code, and how it’s sublime and requires a gift. Now, I don’t know about you but that doesn’t sound like an effective way of learning to me. Like leaving your children outside over the winter it leaves something to be desired in terms of success rate.
Now I too am a firm believer in the 10.000 hour rule, but you have to remember this: Those are supposed to be 10.000 hours of focused training under regular guidance and with practical instruction from a trained professional. So let’s not ask fresh out of school developers to start from zero. Let’s not leave them out in the cold by themselves.
As such I started thinking about some hands on rules for how I myself write and evaluate code once the prime directive of correctness has been satisfied. The idea being that there is a developmental desert between learning syntax/semantics of programming and the fluffier talk of code smells and architecture. A place for truly hands on advice and quantitative rules.
I have found eight so far and in no particular order, this is it:
1. Never return null.
”The supreme art of war is to subdue the enemy without fighting.”
Null is a value, not an error. This should be second nature to any java developer but it is not unusual to find a method returning null on failure, just to have the caller dereference that same null object just lines above in the same file. To be honest I understand. Null checking is crazy hard. That’s why you should not do it. Don’t fight. Let the compiler fight for you.
Exceptions are for errors. Make the effort to create an exception for your package and anyone using it will be forced to deal with any and all failures.
Now, exceptions are expensive goes the old adagio. And it is true. Same goes for the alternative phrasing ”Exceptions are for exceptional circumstances”. Also true. The way to deal with it though is like the old style iterators, add a separate check method such as hasNext() to prevent any expected exceptions. The solution is in no way to skip them for performance critical code.
Now, all this quickly becomes very touchy feely and so that’s why we focus on this simple rule: When you see return null, just say No.
2. Sometimes you need to return null.
“I call it my billion-dollar mistake. It was the invention of the null reference in 1965.” — Tony Hoare
Null is a value. And rarely but sometimes null is valid value. Thus sometimes it is ok to return it. But if so you should make sure when you return it that it is impossible to break the program by not checking for null. The most common case I’ve found for this is html template systems that cannot handle exceptions and uses null to make rendering decisions.
I’ve made it habit of naming the method to make it painfully obvious that it returns null, e.g. getUserProfileOrNull() but it is admittedly a matter of taste.
3. Use static methods if you can.
” Your programs have two essential natures. One part of your program is about moving stuff around, it is machine like. The rest of your program should be logic driven.” — Rich Hickey
Objects are great but I can’t for the life of me understand why you would want only objects. The world becomes such a weird place.
Forget that. Static is your best friend, get to know her. Methods without state are easier to reason about, they are easier to debug, they are thread safe and as a result, they produce less bugs. This rule might also be called, more logic, less machine.
As a side note, you can of course create non-static methods without state as well as static methods with, but I’ve found it helps keeping those member variables from creeping in.
4. Minimise the number of local variables per method.
Pick up any code style book, take up any course, listen to any conference talk. The same thing comes up every time. For the love of god, and all that is holy, do not write impossibly long methods.
At the same time you can open any decent size project and there it is. The hair ball, the freak show, the unmentionable.
Clearly the message isn’t being heard, and no one seems to care. We’ve been going through the same motions for decades now without improvement.
On the other hand, I’m not so sure it’s the number of lines that is the problem. The problem as I perceive it, is the mental strain of reading those lines, and the mental strain depends much more on the number of variables at play than the raw number of lines.
So I suggest this, write however many you lines you want but you can only have six variable objects per method. If you have two input variables, you may create four local ones inside the method and so forth.
If you find yourself with more, you have two choices as I see it. First, you may do the obvious and split out methods for subtasks so that any local variables related to that task do not count anymore. Secondly, you may try grouping variables together in classes. A username object and password object might become a Credentials object for instance.
Now that I think about it, yes: Obviously member variables count against the deadline as well. So another reason to review rule #3.
5. Do not create invalid information.
Writing this I have a very specific case in mind but I also have an eerie feeling it happens in other cases.
So imagine this. Someone sends you a file with dates and values. The dates are on the form “2015/04/20”. It is a date after all and as such you want it in a useful form, so you use a date format to parse it into a java.util.Date object.
Six months later you get a bug report from Australia that the dates are displayed wrong in your app.
If you have ever worked with time as a variable this might sound familiar and you may know that the 20th of april in the default UTC timezone is not always the 20th of april on the other side of the planet.
The actual bug though is that you created information where there was none. You have gone from an object with day-precision to a new object with millisecond precision. Depending on how you count you have added about 26bits of (incorrect) information. Quite an engineering feat. In order to fix it you might even try to add a timezone object. Another couple of bits that wasn’t there at the beginning.
Now, unfortunately the solution isn’t very clear but I have found that especially when it comes to dates, keeping muddy information muddy as long as possible is a much safer way to work. Unless there is a very clear protocol on what the date means, store it as a tuple of ints and don’t convert it to anything else until you really need to. If it’s going to calendar program, a Date object might be right. Perhaps it’s going back out into the wild as XML and in that case you’re already home.
Now as I mentioned, time is a very specific case and it is common enough that it bears mention just as such. However, it is likely this happens all the time under all sorts of circumstances. For example, user objects tend to get extended with a bunch of extra features never supplied by the user themselves. So, if you are not guaranteed to have my aunts phone number, maybe your api should reflect that by storing it separate from core properties such as email and password, and force the client to check if data is present before reading (see rule #1).
6. Never end a classname with -er
What does a ModelManager do? How about if I name it ProductCatalog. Better right? And now, if it starts doing geolocation calls you know something is not quite right. When first I got this advice myself not very many years ago I felt immediately attached to it. I couldn’t really say why though.
I still don’t have a firm understanding of it but here’s my best guess: The suffix -er implies an active agent. A machine (See rule #3 again). It screams complexity and brittleness. Like it could run away and do something on it’s own. I usually visualise FoobarManager as a Rube-Goldberg machine. Little fans and spoons and pendulums moving ping-pong balls around with no margins or error handling. A scary thought.
These names also usually gives little or no hints to the purpose of the objects existence.
Instead, better alternatives are almost always passive in nature. You know a catalog does not take autonomous action. If it does, it is clearly not a catalog. Analogously, a stream or a pipe is better than a writer. A client or a factory is safer than a manager.
The last advantage to this is you discover unnecessary complexities when you’re forced to split the manager’s different responsibilities into one factory, one repository and one service client.
7. Always put any relevant values in your log message.
A colleague of mine did this exceptional experiment once. Our group of ten-ish people was split into pairs and we each got a java program and an ant file for building and running it. We were told the program was faulty and would crash and it was our job to figure out what the problem was as quick as possible. All groups, we were told, had the same error.
Now me and my partner had barely sat down and started looking at the code when the first group had found and fixed theirs. Shortly thereafter two more. After five or ten minutes another couple of groups started reporting success. Another ten minutes passed before the test was aborted and results were gathered. We had barely found the part where the crash occurred and had concluded that an http request somewhere had failed.
What my colleague had left out was that some groups had an error message that included the failing request url, which one could clearly see was misspelled. Another set of groups only got a message that an http request had failed and the third set of groups just got ”Error!”. Needless to say I was in the last group. More importantly the results were perfectly sorted. Those who got a full error message solved the problem immediately. Those who got the ”failed request” were significantly slower and none of us who got ”Error!” even made it over the finish line.
Since that day I always put the input data of whatever my method tries to do in my error messages.
8. Don’t put set methods on your class.
This might also have been called Don’t allow half baked instances.
Essentially what it boils down to is making sure an object is valid directly after the constructor or the factory method is called so client does not need to do anything else.
If some part of the creation failed, a checked exception should be thrown and it should include information on whatever was missing. If your constructor is too big, you likely have too much information in one class. Fix by splitting or subdividing into components.
This is actually one of the less obvious rules. It’s not always clear what constitutes a fully formed instance. If a user object misses a phone number, is that an error? This takes some thinking but trust me, it is worth it in the end. The alternative is you get an error way later in the process when some random code tries to format that required phone number and a null pointer exception ensues (See rule #5).