There’s only one occasion when you do NOT want to hear these words and that is when someone is looking at your code. Sure, you are really proud of it and it might even function properly. But do you ever wonder what another person would think of the code you write? Will they be able to grasp the monumental idea behind it without a debugger and a bottle of Jack Daniels?

I’ve worked at several software companies and each of them had different coding conventions and guidelines for their developers. Some required adding comments as often as possible, while others believed that the code in general, and naming of methods and variables in particular, must be self-explanatory.

Unfortunately, some laws are made to be broken and on so many occasions I would find myself staring helplessly at someone’s method dragging on for several screens, with a dozen nested if/else and some recursion thrown in for good measure. Code like that is as fragile as an ancient Chinese vase - even a gentle fart can break it into pieces.

My personal opinion is that a well-written class should be as readable as a page in a book.

A good author would never name characters “a” or “tmp” or “the_guy_who_constantly_puts_on_his underwear_inside_out”. The author would give them short but unique and obvious names.

And if the book has chapters, then those will not be called “Stuff”, “More stuff” or “Stuff but not the stuff from previous chapter”.

Finally, if a chapter is called “The wedding” then I guess it’s logical that the chapter would describe the wedding only and not birth, adolescence, wedding, divorce and miserable lonely death all at the same time.

Writing clean and understandable code is even more important now that the open source concept has become so popular. Having done a great deal of code reviews over the last few years, I started noticing some common patterns in the mistakes that developers tend to make. Here are a few of them, followed by my humble suggestions on how to improve on those.

1. Too long method/function that does several different things at once

Rule of thumb - function should have one and only one purpose (it’s fine to call other methods from more complex ones like constructors, of course). If it does several things, then it should be broken down into several methods, each correspondingly named.

If you need to create a form of visual controls, make one method for a form and a separate method for each control.

If it’s a call to a REST API, make sure you don’t handle the response in the same method - create separate handler methods.

It’s a very common mistake to have, for example, a class constructor that starts off being small and pretty but just keeps on growing as the class itself is being expanded with new features. It should always be possible to find patterns in the code (unless you are writing a program that emulates the work of our government) and extract those into separate methods.

Look at this constructor that was probably conceived as a well-meant child of some fearless developer but ended up as a fat boy bullied by other (leaner) methods:

Here's the same guy after refactoring therapy (yes, only two lines of code in the constructor):

2. Too short or excessively long names of variables and functions

It should be instantly obvious what a method does, or a variable stores, by the name alone. No “run”. No “calculate”. No “a1”, “a2” and “a3”. For a method name, a verb + noun pair works just fine for most of the cases: calculateTotal, getAverage, setColor, saveContent.

If it’s a method in a class then you should consider it in conjunction with the class name. For example, if you have a class called TextBox then it’s ok to call its method setValue, because textBox.setValue() is very obvious.

If it’s a method that returns a boolean value, start its name with “is” or “are”: isInitialized, isNegative, areEqual etc.

Event handler? Use “on” + event name  (optionally with event target object): onButtonClick, onClose, onAfterRender.

And yes, if you have to call your function  “setValueInTheTextBoxThenSetFocusButOnlyIfValueIsNotEmpty” then your method is doing too much. Goto Problem #1.

3. Putting contents of the entire method inside an “if…” statement

This is a very common and widely-used case. You want the method body to be executed only under certain conditions, so you put the entire code inside an if statement, then you have some more nested if-s inside, and finally end up with some hardly edible spaghetti code on your plate.

Basically, if there’s not much or nothing that happens in the else part of your if statement, then it makes sense to revert conditions, verify the else part first and exit the function before the main code of your method kicks in:

No changes? Do nothing.

Not enough rights? Tell user about it and do nothing.

None of the issues above? Save the changes and then finally do nothing.

This is also much more readable because the logic flow resembles the way our brain works (I'm referring to the "do nothing" part, of course).

4. Else/return overkill

There are several obvious issues here - the if…else… statement could’ve been replaced with a switch… case… and getLaserSwordColor should’ve been called only once with its value assigned to a local variable.

But another, not-so-obvious issue (and the one I’ve seen so many times) is that you don’t need else here at all. The main point of the ..else if.. statements is to skip verifying further conditions once the one that returns true is found. But if you exit the function (call return…) inside each if statement anyway then you won’t need else if, right?

Here’s the function doing the same thing, but in a slightly more readable way:

5. Too many conditions in the same “if…” statement

Is it possible to instantly understand everything this code does? Does it look maintainable? What would you do to yourself should you be asked to make changes to this code? No wonder we, developers, are considered to be alien creatures by regular people who call us bad names like “nerds” and “wise-asses”.

So what should you do to avoid heavy conditions? Use nested if’s and extract conditions into methods with self-explanatory names and use those instead - this will make your code look much better.

If we expand the avenger class with some nicely named methods returning true/false based on characteristics of our heroes, then the piece of code above can be rewritten into something like this:

Wait, what?! Is it really the same method? Now this code can actually be read by a non-technical person who might even understand its purpose. Not only that, but we also implemented some of the nice practices we discussed earlier on, namely used self-explanatory method names (isMale, isOld etc.) and got rid of nasty else-s by exiting the method once it has fulfilled its purpose.

However, this kind of refactoring might shatter your godlike status in your spouse’s eyes, so use with caution.

Of course, these few suggestions are just a tip of the huge iceberg called “refactoring”, but they are very common ones. So do yourself a favor, be a nice person and keep in mind that the code you write may have to be read and maintained by other people in the future.

If you want good karma, write clean code. Turn your classes into lyrics. Make people name their kids after your variables. Have them read your code before bedtime to suddenly realize the sun is rising. And remember what they say about developers who write long functions: they are probably compensating for something.