Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

For someone who doesn't have much real world experience yet, the notion of maintainable code is a bit vague, even though it follows from typical good practice rules. Intuitively I can tell that code can be well written, but not maintainable if for example it hard-wires information that is subject to constant change, but I still have a hard time looking at code and deciding if it's maintainable or not.

So the question is: what hurts maintainability? What should I look for?

share|improve this question

We're looking for long answers that provide some explanation and context. Don't just give a one-line answer; explain why your answer is right, ideally with citations. Answers that don't include explanations may be removed.

2  
@ChrisF Opened a meta question. Would love to hear a more elaborate response. –  EpsilonVector Dec 7 '11 at 6:43
show 6 more comments

17 Answers

up vote 85 down vote accepted

There are many aspects to maintainability, but IMO the most important are loose coupling and high cohesion. Essentially, when you're working with good code, you're able to make small changes here and there without having to keep the whole codebase in your head. With bad code, you would have to take more things into account: fix here, and it breaks elsewhere.

When you have 100+ kLOC of code in front of you, this is crucial. Oft-mentioned "good practice" rules like code formatting, commenting, variable naming, etc. are superficial compared to coupling/cohesion issues.

The trouble with coupling/cohesion is that it's not easy to measure or see quickly. There are some academic attempts to do that, and maybe some static code analysis tools, but not anything I know of that would immediately give a reliable estimate of how hard time you're going to have with the code.

share|improve this answer
3  
@Daniel B: Yep, it's the same thing why managers measure "hours of work" or "lines of code" or "number of bugs". Few of them really believe that such metrics are very meaningful, but it's easy to measure, so let's pretend it's better than nothing... –  Joonas Pulakka Dec 5 '11 at 8:23
17  
+1, very true. That's one of the things I like about Test Driven Design, it lets you know the state of coupling/cohesion. If you need to start each test with 10 lines of mocking/stubbing, you might have a problem with these things. –  Fredrik Dec 5 '11 at 8:31
1  
+1 for 'being able to make changes without keeping the whole codebase in your head'. Loose coupling and high cohesion are part of it, as are some other suggestions in other answers –  Jaap Dec 5 '11 at 13:59
3  
-1. Coupling and cohesion are extremely subjective and too much emphasis on them often leads to ravioli code, where the individual parts are trivial but it's almost impossible to see how they fit together. –  dsimcha Dec 6 '11 at 19:53
3  
I have to agree with @dsimcha. Loose coupling sounds like a good idea on paper, but in practice it turns your system into a maintainability nightmare once it grows past a certain size, for precisely the reason he stated: you have no clue how things fit together. For example, if you've got an object of class Foo, and you call Bar on it, you can tell your IDE to find Bar and you know exactly what's going on. But if you have a more loosely coupled IFoo reference, and you call Bar on that, it can require tons of searching to find which class you're dealing with so you can look up Bar. –  Mason Wheeler Feb 19 '13 at 17:00
show 4 more comments

Duplicated code :

Copy Paste is most probably the single most expensive operation when it comes to maintenance costs of programs.

Why bother move this code to a common component, i'll just copy it over and be done with it Yes... the programmer probably saved an hour of work or so doing things this way. However later comes a bug and... yes of course it gets fixed in only half of the running code. Which means that later still a regression will be logged resulting in yet another fix OF THE SAME THING.

This one sounds obvious but in the grand scheme of things it is insidious. Programmers look good cause they get more done. Dev team manager look good cause they delivered on time. And since the problem is found only much later blame never befalls on the real culprit.

I lost count on how many times I had to halt a release because of such "regressions" Just now I lost one full day tracking a fix that was already done, get it fixed again and pushing the new build through the hoops. So yes, one hour time for one dev six months ago (approx CAD 40$) just costed one full day for 1 senior dev + half day for one junior dev + one full day delay in a project already late....

you do the math....

so next one that pulls this on me I swear he'd better run fast caus'i'll be right behind him !!!

share|improve this answer
30  
On the other hand, when you try to hard to avoid duplicated code, you might create unwanted dependencies (tight coupling) that hurts maintainability. The big questions is if two parts of code are "accidentally similar" or "similar on purpose". In the first case, it's perfectly ok if one copy is changed and the other one untouched. For example, printing delivery notes and printing invoices might initially be identical, but later, prices are ommited from the delivery notes. In the later case, changing one copy means that all other copies must be changed as well. That's when it becomes an issue. –  user281377 Dec 5 '11 at 8:51
2  
"logged resulting in yet another fix OF THE SAME THING" - Almost accurate, but you will actually get "Two or more DIFFERENT fixes OF THE SAME THING" –  NWS Dec 5 '11 at 13:11
show 6 more comments

While code which breaks the rules in the other answers is certainly worse to maintain, all code is hard to maintain, therefore the less of it you have the better. Defects correlate strongly with amount of code, so the more code you have the more bugs you have. Once code get over a certain size you can't keep your whole design in your head anymore and things get much harder. Lastly more code eventually means more engineers, which means more cost, more communication problems and more management problems.

so in a nutshell, any superfluous code is a maintainability problem

share|improve this answer
4  
Analogous to the hardware problem: "reliability is inversely proportional to the (of the order of xx?) number of components"/ "a mechanical device is just as bad as the number of moving parts it has". –  Kris Dec 5 '11 at 10:57
1  
"the less of it you have the better" - logical extrapolation is to have NO code and thus solve ALL your maintenance issues. Seriously, though, your nutshell summary is an argument for keeping code as DRY as possible; and for refactoring as part of the development and maintenance cycles. –  Chris Walton Dec 5 '11 at 17:12
1  
That's exactly why domain specific languages should always be used. A typical DSL implementation alongside with the code written in it is in order of magnitude smaller than an equivalent ad hoc code in any of the so called "general purpose" languages. –  SK-logic Dec 6 '11 at 8:36
2  
@ChrisWalton: And don't solve problems you don't need to solve, expend too much effort/time on future proofing, etc. –  dsimcha Dec 6 '11 at 19:48
show 2 more comments

The only true way of learning what is maintainable code is to get involved in maintaining a large codebase (both bug fixes and feature requests).

You will learn about:

  • loose coupling and high cohesion
  • duplicated code
  • regression tests and refactoring: with an efficient regression test suite, you feel more confident in changing things.
  • observability and error logging: if the software traces what is going wrong, the bug is more easily located.
  • accurate documentation: not the kind that is written once at the beginning of the project and never modified.
  • undocumented obfuscated code taking the form of a supposedly brilliant design idea or optimizations ala Fast inverse square root.
  • oral tradition transmitted from one generation of developers/maintainers to the next one.
share|improve this answer
1  
@SteveMelnikoff - This is ironical: oral tradition on a software project is a barrier against maintainability. –  mouviciel Dec 5 '11 at 19:08
show 2 more comments

I believe that code that isn't Unit Tested hurts maintainability. When code is covered by unit tests, then you can be confident that when you change it you know what you are breaking because the associated unit tests will fail.

Having unit tests along with the other suggestions here makes your code robust and you make sure you know that when you change, exactly what impacts you're having.

share|improve this answer
3  
Some code is hard to unit-test, and trying to force it to be testable may not always be worth the effort. –  tdammers Dec 5 '11 at 15:19
show 4 more comments

In my experience, a major factor in maintainability is consistency. Even if the program works in strange and absurd ways, it's still easier to maintain if it does it the same way everywhere. Working with code that uses different naming schemes, design patterns, algorithms etc. for similar tasks, depending on who wrote it when, is a major PITA.

share|improve this answer
add comment

I think that maintainability is also tightly coupled with problem complexity, which can be a subjective matter. I've seen situations where the sole developer was able to successfully maintain and consistently grow a large code base, but when others step into his place, it appears an unmaintainable mess -- just because they have quite different mental models.

Patterns and practices really help (see other answers for great advice). However, their abuse can lead to even more problems when the original solution is lost behind facades, adapters and unnecessary abstractions.

In general, understanding comes with experience. A great way to learn is to analyze how others have solved similar problems, trying to find strong and weak points in their implementation.

share|improve this answer
add comment

Almost everything which violates some good software development principles hurts maintainability. If you look at some code and want to assess how maintainable it is, you can choose some specific principles and check how much it violates them.

The DRY principle is a good starting point: How much information is repeated in the code? YAGNI is also very helpful: How much functionality is there which is not needed?

If you do some research on DRY and YAGNI you will find other principles, which are applicable as general software development principles. If you are using some object-oriented programming language and want to be more specific, the SOLID principles give some good guidelines for good object-oriented design.

Code which violates any of those principles tends to be less maintainable. I put an emphasis on tend here, because – generally spoken – there are situations where it is legit to (mildly) violate any kind of principle, especially if it was made for the sake of maintainability.

What also helps is to look for code smells. Take a look in Refactoring - Improving the Design of Existing Code from Martin Fowler. There you can find examples for code smells, which sharpens your sense for less maintainable code.

share|improve this answer
show 1 more comment
  1. Lack of proper unit-tests
  2. Lack of proper integration tests
  3. Lack of proper benchmarking performance tests
  4. YAGNI

3 and 4 are, in my experience, the most common offenders in real-life systems. Not having a proper performance/loadtesting suite makes it impossible to benchmark the impact future changes will have on performance and stability and it will cause degradation much quicker and force expensive refactorings more often. YAGNI (what's called "waste" in the Lean vocabulary) will make a system much harder to refactor and maintain as a lot of time will be wasted (pun intended) on maintaining code and backwards compatibility for features that aren't even used.

share|improve this answer
2  
I think you have YAGNI backwards. YAGNI is the principle to not write unused features, not the unused features. –  Kevin Reid Dec 5 '11 at 17:15
add comment

I have a couple of "nightmares" that although I'd never put them on the top of this list, they have definitely caused some struggle.

The mega object/page/function: (The so called "God Object" anti-pattern): Putting everything in one object's method (mostly Java world), page (mostly php and asp), method (mostly VB6).

I had to maintain code from a programmer who had in one php script: html code, business logic, calls to mysql all messed up. Something like:

foeach( item in sql_query("Select * from Items")) {
<a href="/go/to/item['id']">item['name']</a>
}

Relying on obscure features of the language that might not be well known or are about to become obsolete.

It had happened again with php and preset variables. Short story: for some settings of php v4 a request parameter was auto assigned to a variable. so http://mysite.com?id=44 would "magically" generate varaible $id with value '42'.

This was a maintenance nightmare not only because you could not possibly know where data came from but also because php v5 completly removed it (+1), so people trained after that version, would not even understand what was going on and why.

share|improve this answer
show 2 more comments

Ironically, explicit attempts to "future-proof" a system often make it harder to maintain.

Not hardcoding magic numbers or strings is one thing, but take it further and you start putting logic into configuration files. Which means you're adding a parser and an interpreter for an obscure new (and probably badly-designed) programming language to your maintenance burden.

Layers of abstraction and indirection added for no other reason that some component might be changed in the future and you don't want to depend on it directly are pure maintenance poison and a prime example for YAGNI. The SLF4J FAQ explains the problems with this all-too-common idea:

Given that writing a logging wrapper does not seem that hard, some developers will be tempted to wrap SLF4J and link with it only if it is already present on the classpath, making SLF4J an optional dependency of Wombat. In addition to solving the dependency problem, the wrapper will isolate Wombat from SLF4J's API ensuring that logging in Wombat is future-proof.

On the other hand, any SLF4J-wrapper by definition depends on SLF4J. It is bound to have the same general API. If in the future a new and significantly different logging API comes along, code that uses the wrapper will be equally difficult to migrate to the new API as code that used SLF4J directly. Thus, the wrapper is not likely to future-proof your code, but to make it more complex by adding an additional indirection on top of SLF4J, which is an indirection in itself.

The last half-sentence hints at the sweet irony that SLF4J itself is a logging wrapper to which this argument applies just as well (except it's making a credible attempt to be the only wrapper you need that can sit on top of and beneath all other common Java logging APIs).

share|improve this answer
1  
I agree with this observation in principle, but it is less clear cut than the others. It is sensible to do a limited amount of future proofing and decoupling, especially in areas that you know are likely to see change. You are quite correct that it is possible to go overboard in this regard. Knowing the right amount to do in a given situation could be difficult to determine. It requires experience and judgement, and reasonable developers will often disagree. –  PeterAllenWebb Dec 5 '11 at 15:59
show 2 more comments

The better approach for maintainability from my experience is to have as little code as possible and shove the work to others (i.e. the software vendors) who do the maintenance for you.

I prefer to have as little custom source as possible and push as much as possible to Excel sheets, XML or property files that represent the business data and provide simple readers or use an ETL tool to do the conversion to application code.

I also try to find something that is standard based (i.e. no single vendor) if possible. However, there are some things we cannot avoid e.g. Spring for dependency injection before JEE6 and Hibernate for ORM before JEE5, log4j/commons-logging/slf4j before Java 1.4. By choosing something standard based, there is a higher possibility of switching providers especially if one becomes defunct or prohibitively expensive and you avoid classpath hassles.

Of course if you're the vendor that is providing the software that's another matter. However, if you are dealing with real life situations sometimes it's better to throw money into the problem rather than skill and time.

share|improve this answer
show 2 more comments

Anything that is not simple to understand.

The most destructive force for any kind of code is maintenance. Unfortunately, maintenance is also absolutely necessary. If the maintainer does not quickly understand the code they will either route around it which is just as destructive or almost certainly break it.

To misquote Einstein: Make it as simple as possible - but no simpler.

share|improve this answer
show 2 more comments

Too many features.

Features themselves by nature hurt maintainability because you need to write extra code to implement them, but an application without features is pointless, so we add features. However, by leaving out unnecessary features you can keep your code more maintainable.

share|improve this answer
add comment

What makes maintainance necessary is that requirements are a moving target. I like to see code where future possible changes have been anticipated, and thought put into how to handle them if they should arise.

I have a measure of maintainability. Suppose a new requirement comes along, or a change in an existing requirement. Then the changes to the source code are implemented, along with fixing any related bugs, until the implementation is complete and correct. Now run a diff between the code base after and the code base before. If that includes documentation changes, include those in the code base. From the diff you can get a count N of how many insertions, deletions, and replacements of code were necessary to accomplish the change.

The smaller N is, as a rough average over past and future requirements changes, the more maintainable the code is. The reason is, programmers are noisy channels. They make mistakes. The more changes they have to make, the more mistakes they make, which all become bugs, and every bug is harder to find and fix than it is to make in the first place.

So I'm agreeing with the answers that say follow Don't Repeat Yourself (DRY) or what's called cookie-cutter-code.

I'm also agreeing with the movement toward domain-specific-languages (DSLs) provided they reduce N. Sometimes people assume the purpose of a DSL is to be "coder-friendly" by dealing in "high-level abstractions". That doesn't necessarily reduce N. The way to reduce N is to get into a language (which may be just things defined on top of an existing language) that map more closely onto the concepts of the problem domain.

Maintainability doesn't necessarily mean any programmer can just dive right in. The example I use from my own experience is Differential Execution. The price is a significant learning curve. The reward is an order of magnitude reduction in source code for user interface dialogs, especially those that have dynamically changing elements. Simple changes have N around 2 or 3. More complex changes have N around 5 or 6. When N is small like that, the likelihood of introducing bugs is much reduced, and it gives the impression that it "just works".

At the other extreme, I've seen code where N was typically in the range of 20-30.

share|improve this answer
show 2 more comments

This isn't really an answer so much as a long comment (Because these answers have already been presented).

I've found that striving religiously for two factors--clean, usable interfaces and no repetition have made my code much better and over time have made me a much better programmer.

Sometimes eliminating redundant code is HARD, it forces you to come up with some tricky patterns.

I usually analyze what MUST change then make that my goal. For instance, if you are doing a GUI on a client to update a database--what do you need to add "Another" one (another control linked to the DB?) You need to add a row to the DB, and you need the position of the component, that's it.

So if that is your bare minimum, I don't see ANY code in that--you SHOULD be able to do it without touching your codebase. This is amazingly hard to attain, a few toolkits will do it (usually poorly), but it's a goal. How hard is it to get close? Not terribly. I can do it and have done it with zero code in a couple ways, one is by tagging new GUI components with the name of the table in the DB, another by creating an XML file--the XML file (or YAML if you hate XML) can be really useful because you can link validators and special actions to the field, making the pattern extremely flexible.

Also, it doesn't take more time to implement solutions correctly--by the time you've shipped most projects it's actually cheaper.

I can point out that if you rely heavily on Setters & Getters ("Bean Patterns"), Generics & Anonymous inner classes you probably AREN'T coding generically like this. In the above examples, trying to force in any of these will really screw you up. Setters & Getters force you to use code for new attributes, Generics force you to instantiate classes (which requires code) & Anonymous inner classes tend not to be easy to re-use elsewhere. If you are really coding generically, these language constructs aren't bad, but they can make it hard to visualize a pattern. For a totally nonsensical example:

user.setFirstName(screen.getFirstName());
user.setLastName(screen.getLastName());

Looks fine--not really redundant at all--at least not in a way you can fix, right? But it causes you to add a line when you want to add a "Middle Name", so it is "Redundant code"

user.getNameAttributesFrom(screen);

Does not need new code for this task--it simply requires that some attributes in "screen" are tagged with a "Name" attribute, but now we can't copy address, how about this:

user.getAttributesFrom(screen, NAME_FIELDS, ADDRESS_FIELDS);

Nicer, a var-args lets you include a group of attributes (from an enum) to gather from the screen--still you have to edit code to modify the types of attributes you want. Note that "NAME_FIELDS" is a simple enum instance--fields in "screen" are tagged with this enum when designed to put them in the correct categorie(s)--there is no conversion table.

Attribute[] attrs=new Attributes[]{NAME_FIELDS, ADDRESS_FIELDS, FRIENDS_FIELDS};
user.getAttributesFrom(screen, attrs);

Now you've got it to where you are just changing "Data". This is where I usually leave it--with the data in the code--because it's "Good enough". The next step is to externalize the data if that is ever needed, so that you can read it from a text file, but it rarely is. Refactorings "Roll up" like this a lot once you get into the habit, and what I just did there^ created a new enum and pattern that will end up rolling in many other refactorings.

Finally, note that good generic solutions to problems like this are NOT language-dependent. I've implemented a no-code-per-loc solution to parsing a text GUI from the command line of a router, updating it on the screen and writing it back to the device--in VB 3. It just takes dedication to the principle of don't write redundant code, ever--even if you have to write twice as much code to do so!

The Clean Interface (Fully Factored & doesn't allow illegal stuff to pass through) is important too. When you factor an interface between two units of code correctly, it allows you to manipulate code on either side of the interface with impunity and allows new users to implement to the interfaces cleanly.

share|improve this answer
add comment

besides the mentioned patterns one of the most important things is

readable code, every single line of code you put in a codebase will be read 100+ times and as people are tracking down bugs you do not want them spend 30 minutes to try and figure out what it does each time.

So don't try to be clever with "optimized" code unless it really is a bottleneck and then comment it and put the original readable (and working) code in comments (or alternative compilation path for testing) as a reference for what it should do.

this includes descriptive function, parameter and variable names and comments explaining WHY a certain algorithm is chosen over another

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.