Even Good Programmers Need To Review The Basics
In twenty years of programming and managing programmers I have seen great programmers and I’ve seen programmers that couldn’t write “Hello World” without a bug. This isn’t about the latter, it’s about real world examples of good engineers making the most basic mistakes (myself included). Here are few coding bad practices I’ve observed in every production environment in which I’ve been.
back end sneaking up front
Everyone knows to keep the interface, processing, and data layers separate and interact only through proper calls. The proper practice is for a function or method to return only the “answer” or to “do the job” and do nothing else. It’s the UI layer that calls the function, gets the return value, and then decides what to show the user. Just yesterday I made a mistake that violated this rule which is also what prompted me to write this blog.
I’ve observed this rule being broken more and more in very subtle ways. Modern IDEs with visual editors have made the line between the UI and processing layers somewhat fuzzy. Yesterday I was writing an input verifier (Java) for a jTextField. The verifier tested the text and if it was good it returned true. If the text was bad the verifier threw up an error dialog and returned false. I was coding in the UI layer, right? I clicked the input verifier method in the visual editor and I was writing code for a UI component. All UI layer, right? Wrong. The input verifier is back-end code and should only return true or false. It’s the focus listeners in the UI that should be reading that return value and throwing up dialogs.
What’s the difference? Reusability. The first input verifier I wrote can’t be used again in any other context. The second version is a nice generic verifier I can use anywhere (and now is in my library). A very simple mistake but I’ve noticed this happening with many good programmers especially when working with visual editors.
multiple returns (greater than two)
This is the practice of putting returns all through a method or function everywhere it’s possible the correct answer is achieved (i.e no further processing is technically needed). For example, a switch statement with 10 cases and 10 returns. Another example is several nested if/else statements where each part of each if/else has its own return. Personally I’m very strict on this one but whenever I’ve pointed this out to another programmer the invariable answer I get back is “only one return gets executed so what’s the problem?”
Two problems,… 1) the potential for bugs in functions and methods with many returns is very high, especially with nested if/else statements. If you enter a series of decision structures you should back out of that series properly and then return – not return in the middle, and 2) code optimization at both compile time and run time is adversely affected. Both the compiler and the CPU like functions and methods that have a single entry and exit point. This facilitates many useful optimizations. In any case using multiple returns is just plain sloppy programming – don’t do it.
hard coding
The classic example of this is writing a string for display to the user (e.g. an error message) and putting the string right into the code. The right way to do this is to place your strings or other literals elsewhere and have the code look-up the values.
Why? So you can change the values later without re-compiling. Anytime you need to go back into the code to change a literal you’re not only wasting time, you risk breaking something. This is probably not important (or even advised) in cases of client side web coding (i.e. JavaScript) because the overhead of the lookup is too high, but in the case of compiled back end code store your literals elsewhere.
not using regular expressions
Here is one that I used to be guilty of but I corrected myself years ago. I’ve seen so many programmers write text processing routines that do character by character searches. Text processing can be done easily by constructing regular expressions and calling the right string routine. Most reading this will say “duh!” but as a programming manager I’ve seen this more times than I can count. This bad habit is easy to explain, regular expressions seem complicated at first (they are not really). A lot of good programmers don’t get past that and never re-think it.
over-complicating the basics
Here’s an example I observed a few months ago. A programmer (a quite good one) was constructing a user preference system that would save and load user preferences from an XML file. It worked very well but it was large and complicated and it began to encompass other information besides just user preferences (application state, OS state, etc.). What’s a better and simpler solution?
We were in Java so the answer was serialization. We had a user preferences object so why go through all the trouble of reading/writing/processing XML? We threw out the XML and replaced it with simple serialization. When the application loads it picks up the serialized preferences object and when it shuts down it serializes that object back to disk. The programmer now only needs to be concerned with managing the preferences object (which he had to do anyway) and not how it’s saved or processed.
The code savings was several hundred lines and the complete elimination of the XML subsystem. The XML solution seemed excellent at first because it was infinitely scalable and could do all sorts of cool processing but it was totally unnecessary, it overcomplicated the application, and it begged for scope creep.
re-inventing the wheel
By now everything even remotely useful has been written by someone somewhere and then probably refined by someone else and posted online. I was once hired on a job to manage a group of engineers. When I was hired they were already three months into writing complex math modules from scratch. Every single thing they were doing was freely available open source and no one even bothered to look for it.
I assume that is a dramatic example (8 engineers wasting 3 months) which doesn’t come up often but I see small examples of this all the time. Personally I don’t write anything I can copy. It’s become standard practice for me before I get into writing any object, function, method, etc. I do a quick search to see if someone else already did it and if it’s freely available.
the most evil – the goto statement
I saved the best for last, the goto statement. I can’t hardly believe this is still an issue but it is. On another job I had a couple years back (again managing engineers) when I was first hired I looked into the code and found goto statements everywhere. They were used by every programmer in the company.
I asked a lead developer why this was acceptable practice and he said the previous manager advocated the use of goto. The code produced in this environment was completely unreadable and they averaged over a dozen reported bugs a day for a relatively small application. I even saw “loops” written purely with if and goto statements (hadn’t seen that since programming on my Apple IIc). I’m not going to explain why the goto is wrong and completely unnecessary. That topic has been beaten to death and it’s been proven wrong beyond a shadow of a doubt both academically and practically. If you’re using goto’s, stop it.
good coders gone bad,… maybe just a little?
In the first example I gave above I caught my mistake right away but it made me think about the fact that even experienced coders let things slip if they don’t stay disciplined on the basics. I also break the hard coding rule from time to time too (just being lazy). Anyone else feeling a little guilty on any of these?
About the author
Alex is software executive with over 15 years experience in designing and building enterprise applications, scientific and engineering software development, and executive management in the software industry. Website: www.amk.cc
may I add this blog post? i will link back to this post.. let me know please, thanks
I don’t know about Amber but I posted this on my own site as well. Please feel free to cross post it and link back to me. http://www.amk.cc/blog/archives/32
Thanks.
A good post outlining some poor coding practices which should be less common than they are.
Not reinventing the wheel is important, but be sure you understand the wheel. I’ve worked on projects where code doesn’t work because “developers” have simply copied code without understanding how the code works with their own code and fits into the project as a whole.
Good point, if I had thought of it at the time I’d have probably added the caveat “but make sure you understand the copied code.” Actually, another common “bad practice” itself could be “copying code you don’t understand.”