Writing Maintainable Code

Here I suggest some guidelines that all programmers, regardless of language, can follow to keep their code maintainable by others.

Introduction

The success rate for software development projects around the world is shameful. Projects are very often delivered late or over budget, which is acceptable because the alternative seems to be not delivering the project at all. Certainly there are things in the development lifecycle that are out of the developer's hands, such as the quality of the requirements. However, one cause of poor software delivery success rates is that applications with large amounts of code can quickly become unmanageable, tangled messes that no one wants to touch. While the steps in this article aren't the only answer to these problems, writing maintainable code will help keep projects moving to completion and through the support phase.

Write For the Person Who Will Maintain Your Code

Unless you work for yourself, the code you write will likely be touched by someone else at one time or another. Just because you felt like you were looking for a needle in a haystack when you were maintaining someone else's code doesn't mean you should force the next person to have the same experience. Instead, focus on things you can do to make your code more readable.

Variable/Method Names

I'm not going to advocate the use of any particular notation or convention. As long as the notation makes sense and is consistent, you're probably fine. What you want to avoid is changing notation dramatically during the course of development work. The reader should be able to get used to the notation, whatever it is, fairly quickly, and changing conventions mid-stream will only cause confusion.

One thing you should focus on is making readable variable, function, and object names. You might not think there's a problem with this:

//s will be some type of saxophoneObject s = this.GetSaxophone();

The problem is that if you have a long function, you will have to remember what "s" is. Instead of doing that, try this:

Object saxophone = this.GetSaxophone();

Now, no one needs to question what "saxophone" is supposed to be.

The same concept applies to method names. The method name should be a short description of the method's purpose, not some arbitrary set of letters. Also, since you are naming functions after what they do, you should ensure that your function names are verbs to reflect their purpose.

Short Methods

It can be difficult understanding what's going on inside many functions, but it's much worse when the function is 2000 lines long. If you're debugging such a monster, you're likely going to forget all of the assumptions that were in place when the function started. You're also likely going to have problems mentally breaking down the function into smaller bits of functionality for debugging. The solution is to explicitly break the function down into smaller ones, ideally groups no larger than your computer screen. (No, this does not give you an excuse to cram as much code into each line as possible.) Your parent function could call these child functions, giving the code reader an easy way to get a high level view of the code's purpose by looking at the parent function, while also allowing the reader to see each component individually by looking at the child functions.

Refactoring

Whenever you're making a change to code, you're probably either making it better or making it worse. If you're leaving code in that no longer works, or are working around code that may or may not work, or are leaving in code that works but has turned into spaghetti code due to all of the changes made, you're making it worse. Take the following example:

if (MySaxophone is AltoSaxophone)
DoSomething();
elseif (MySaxophone is TenorSaxophone)
DoSomethingElse();

By this point, you should be thinking about refactoring this code. What happens if you add a BaritoneSaxophone type? How about SopranoSaxophone or BassSaxophone? If you're working in an object-oriented language, you should consider moving this method into the base Saxophone object and override it in the specific saxophone types when needed. Otherwise, you could run into a rather large if/else chain.

If you're thinking that this example isn't so bad, you're right. If your application is fifty lines long, having one awkward chain of if/else statements isn't going to make or break your application. When you apply this idea to an application that has several hundred thousand lines of code, though, it should be fairly obvious why it's important to keep on top of the little things before they become big problems.

Real Fixes vs. Quick Fixes

Related to refactoring, you as a developer should always be looking to fix the cause of the problem, not to fix the symptoms. For example, if an application blows up whenever it goes to the database and gets a string when it expects an integer, many developers I've encountered will trap the data and try to turn it into an integer the best they can. That will make the application worse. The correct solution would be to find out why the non-integers are being put into the database in the first place. Is this the cause of the bug? Do you misunderstand what this field is actually being used for? Or is the actual problem something else entirely? Without the answers to these questions, you're at best creating additional confusing code for the next person to clean up and at worst causing more bugs than you are fixing.

Commenting/Documentation

You might be surprised to see that I haven't mentioned comments or inline documentation yet. Comments can be useful at times, but one should never write a comment to explain an unclear block of code without first making some effort to make it readable. Your goal should be allowing your reader to understand what you are doing the first time through your code. Use comments when they get you further towards this goal.

Keep Code Testable

There have been several studies done stating that the sooner an error is found, the cheaper it is to fix. Running the application and testing each possible scenario can be time-consuming, and regression testing large applications can be expensive and unpleasant. Fortunately, you can write methods to test your own code, helping to eliminate errors before they are found by testers or end-users.

Write Unit Tests

Unit tests are essentially bits of code that test individual components without needing to step through each line manually. Here is an example of a method that could use a unit test:

Decimal CalculateDiscount(Int32 age, Decimal amount)
{
// Code to calculate your sales discount here // based on the age of the customer and the size of the purchase
}

There are two important advantages to being able to test your code like this. The first is that you don't actually have to start up your application to test this method. If this were contained within an e-commerce website, you might have to log in as three different users and make three different purchases in these amounts to test the same scenarios, which would be very time-consuming. Unit tests are much faster to run. The second advantage is that these unit tests would continue to exist in your project (though preferably within their own container so the code wouldn't be pushed to production). They could be run periodically, which greatly reduces the need for manual regression testing.

A further discussion of unit testing is beyond the scope of this article, but entire books have been written on the subject. There are also multiple frameworks available in multiple languages to help you get started.

Write Once, Use Most Places

Another benefit to breaking your code into smaller pieces is that the components can be reused. Using the example in the previous section, if your application needed to get the discount amount in multiple places, you can simply call your original function in each place and can reasonably expect it to work. Having said that, don't take this to extremes. There are scenarios in which you may be tempted to push the limits of code reuse. Create new functions or objects when necessary to keep your code simpler.

Use Exceptions Wisely

Exception handling (preventing the user from seeing their application crash) is, in general, a good thing. Most undesirable situations, such as the user entering inappropriate values, should be anticipated and handled properly. However, exception handling can be overdone. There are scenarios where it is appropriate for your user to see a generic error screen (which of course sends you, the developer, an e-mail telling you an error occurred and hopefully what caused it). To see why, let's look at another saxophone example:

Unfortunately, I've seen multiple applications where the exception handling code (in the "catch" section) does nothing whatsoever - the application just moves along without giving the user any indication that there was a problem. The reasoning behind that approach seems to be that if the user doesn't see an error screen, there will be fewer complaints. While this may be true, the result is that you (as the programmer) won't get helpful information to solve your problem. The user won't know a serious error occurred. If they notice anything at all, it will be that the application doesn't work as they expect, so you may get complaints about your user-interface, faulty data, or maybe 100 other things, none of which point directly to your problem.

In this case, a saxophone without a G key wouldn't be of much use to most players, so hiding the error from the user isn't a good idea. If saxophones often didn't have G keys, I would suggest using local error-handling code to show the error to the user and tell them ways they might solve the problem themselves. Since saxophones shouldn't be missing G keys, you should strongly consider skipping the localized error-handling logic completely. Let your global routine handle this error, since having a saxophone without a G key is serious enough for the user to take notice and for you to be notified immediately.

Focus on the Big Picture

Ultimately, the goal is to allow the person maintaining your code to focus on the problem at hand, rather than sorting through your mess. The usual methods of making code maintainable, such as naming conventions, patterns, etc., are only useful if they give the next person a way to understand what you were trying to do. So don't feel like you have to pick the "right" naming conventions, patterns, etc., as long as you are continually making an effort to make your code readable. And remember, if another programmer doesn't understand your code, it's probably your fault.

Further Reading

If you wish to read further on the subject, I highly recommend Steve McConnell's book "Code Complete", available from Microsoft Press. If you ignore the fact that the title is grammatically awkward, it is a very good book, filled with practical advice every programmer should consider while writing code.

Share

About the Author

Scott started his career in the working world as a band instrument repairman at a music store in the Chicago area. Not long after starting repairing, Scott started teaching himself various web technologies (C++, HTML, CSS, JavaScript, and eventually ASP.NET with C#) and soon discovered that programming matched his interests and abilities much better than repairing band instruments. After working in sales and web development for a music store, he started working for Adage Technologies, a web development consulting firm specializing in custom software solutions. He is currently a lead developer/project manager for a multi-phase, multi-year project for a non-profit firm. He spends most of his free time studying for his MBA at the Kelley School of Business at Indiana University.

I'm a firm beleiver in Altzheimer's Law of programming: You WILL forget why you wrote the code that way and what those variable and method names mean.

With me it kicks in at about 8 weeks. I go back to code I wrote and unless I've been very careful to keep it well structured and/or obvious and/or well commented I might struggle to understand why I did something that way, or even what I was trying to do

While you are correct in noting that short variable names may be confusing when they are used over a wide scope, I find that very short (generally 1-4 letter) names are useful when the meaningful scope of variables is very narrow. To my mind, the fact that a variable is given a 'long' name should suggest that the variable has some long-term meaning.

If those three statements are the only ones that use the variable, why give it a long name? What's important about the variable isn't what it's called, but what's done with it, and that IMHO can be more readily ascertained if its name is short.

This is the motto that all developers should live by. Spending that little bit of extra time to put in a meaningful variable or method name, to break a method into an additional 5 methods or adding that bit of additional white space (not mentioned in the article by the way ) - these are the things that are going to save time in maintenance and readability later.

Nitpicking on your example (sorry), but tS can mean any number of things.
In the context of music, we have
- TimeSignature
- TimeStamp
- TimeSpan
- TheScore (ok, I'm grasping at straws here, but you get the drift).

It may be that the scope of the variables at the time of writing are small, but that does not mean that this is always going to be the case.

and at a later date, we wish to add the saxophone to a concert band but as a flute :-

concertBand.Add(saxophone.Transpose(InstrumentType.Flute))

Not only does this make the code a lot easier to read, but a future enhancement will also be easier to implement correctly.

Visual Studio also provides intellisense, of which I am a very big fan of when put into manual mode (you have to press CTRL-K). So with the above example, you only have to type the entire variable name once and use intellisense after typing the start of the variable name.

One of the standards that we have implemented at work is that all variables must have a meaningful name and abbreviations are not allowed (except for technical key words like XML or HTML). It does take time for some people to implement, but in the end, everyone is better off.

It may be that the scope of the variables at the time of writing are small, but that does not mean that this is always going to be the case.

If the code later changes to give the variable a larger scope, I'll change the name as appropriate; the issue isn't just that I dislike typing, but rather that if I have many consecutive pieces of code like:

which are conceptually similar, but look for different delimiters and do different things with them, it's clearer to use a short generic name than to create many different variables LocationOfColonInInstrumentName, LocationOfCommaInComposerName, etc. If the the longer names were used, someone who inspected the code and was considering refactoring to use a munging function would have to worry about whether the result of InstrumentName.IndexOf() might be used somewhere else; in code like the above, it's pretty obvious it isn't.

Incidentally, what are your thoughts about variable names for general-purpose parameters? For example:

Function Hex2(N as Integer)
Static Hex2Results(255) asString
If Hex2Results(N) Is Nothing Then ' No need for threading interlock; the worst that can
Hex2Results(N) = N.ToString("X2") ' happen is we create an extra string.
EndIf
Return Hex2Results(N)
End Function

Is any advantage gained by substituting something longer for "N"? (Incidentally, a function like the above will likely end up wasting a small amount of storage holding strings that are no longer used except in the Hex2Results array, but it should reduce wear and tear on the garbage collector by avoiding a string allocation every time it's used).

Also as you might have noticed in the code block, use the constant before the variable in an if block. This might not seem logical, but it prevents expressions to always result in a true value if you forget one '=' sign.

"Constant before variable" used to be good advice in C and C++, but not in C#. In C#, the conditional expression in the if-statement has to be of type boolean, so if you accidently type "if (saxophone = null)", the code won't compile.

Yeah and compiler warnings if set high enough would find it as well, but if you code it the way he was saying (constant before the variable) then you won't have to think about whether your in C# or the next guy has compiler warnings set high enough.

I always write a block, but I don't know whether it deserves to be a recommendation or just a suggestion.

The "constant before the variable" concept is pointless (even when writing C/C++); it only applies when testing an Rvalue and an Lvalue, not when testing two Lvalues or two Rvalues, and if the developer can remember to do it then he can remember to use the correct operator. I once worked for a company that had it in their coding standard, but even the "guru" (who probably wrote the standard back in the '70s) agreed that it wasn't worth the trouble.

I agree with the other people as well. Most people skip coments, they think their code is readable. I have just had to go into some code that people thought was readable, it was not to me. Rememebr that others even yourself has to go back into the code months later. At least write what you thought the code should do. even if the coments match that even better.

You don't get the point of documentation, yet you try to explain what maintainable code is. Documentation is for documenting WHY you wrote the code the way you wrote and more importantly: why you didn't pick alternatives to solve the same problem.

That information can't be determined from code, as code is just the executable form of what you had to write. Documenting what the code does is not really useful, the documentation should help the reader understand WHY the code is constructed the way it is. Comments serve the same purpose, on a much smaller scale. So comments about "Increase counter" are useless, as the code already shows that. Comments about "We don't call Foo here because that would create problem blabla because... " is very helpful so a maintainer won't add a call to Foo there _OR_ would investigate if the original writer indeed was correct about problem blabla.

Not having that comment there would make maintaining code much harder, and not having documentation about why code is the way it is is making maintaining code VERY hard.

Therefore, this whole article is pretty much useless and actually harmful, as the most important point of maintainable code is documentation about why it is the way it is: the code is just a result of that, not the source. 'Sourcecode' might suggest it is the starting point of something, but it's actually the end point.

--
Lead developer of LLBLGen Pro: http://www.llblgen.com
Only the true wise understand the difference between knowledge and wisdom.

You missed the point I was trying to make in the article. I will reword the commenting section a bit, since I was not trying to make the point that comments were unneeded. I agree completely that comments can be useful at times, as long as they're not used as an excuse to avoid making improvements to the code itself.

While I whole heartedly agree that superfluous comments are a waste and a distraction, I feel many people who believe their code is totally readable and transparent still under comment. On class declarations, give a short concise description of the class and the purpose it fulfills, don't make me read through all method declarations or, worse still, resort to crawling through the source to figure it out. For a substantial function, tell me what algorithm is being used. I don't want to have to read the code and try to guess. I may not be familiar with all the algorithms available or they may be similar enough that it might take a minute or two to think through it. Just tell me so I know at a quick glance and can move on feeling confident we're on the same page. If there are exceptional conditions handled, indicate them and give a how and why.

And if it's important enough to comment, keep them up to date with revisions to the code. I've heard people say they don't comment their code because the comments are always out of date. That's just an excuse for laziness. It's part of the job so do it.

You measure democracy by the freedom it gives its dissidents, not the freedom it gives its assimilated conformists.