75

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?

7
  • I find it sad that this question was closed. I'd appreciate at least a comment on why it does not meet the "six guidelines". To me this question is "practical, answerable, and of some educational value to the greater community".
    – sleske
    Dec 6, 2011 at 13:14
  • @sleske I agree. This seems like exactly the kinds of questions this site was opened for. I'm a bit puzzled about this too. Dec 6, 2011 at 19:27
  • @sleske - Ask a question on Meta to draw some attention to the question. It had 3 community close votes before I added mine (plus a flag as well). It already has one reopen vote, if you can get 4 more people to agree it will be reopened.
    – ChrisF
    Dec 6, 2011 at 22:49
  • 1
    @MichaelBorgwardt - sorry. I misread your comment. The fact that it has 18 answers is an indication that it's not constructive. It shows that there are many equally valid answers to the question. You'll say that that's self evident, but it does mean that the question isn't a good fit for the Stack Exchange question and answer model.
    – ChrisF
    Dec 6, 2011 at 23:16
  • 3
    @ChrisF Opened a meta question. Would love to hear a more elaborate response. Dec 7, 2011 at 6:43

17 Answers 17

97

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.

9
  • 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... Dec 5, 2011 at 8:23
  • 21
    +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, 2011 at 8:31
  • 3
    Coupling and cohesion are often thought of more easily with compartmentalization (but must be thought of as their own goals). Dec 5, 2011 at 17:32
  • 6
    -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, 2011 at 19:53
  • 5
    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. Feb 19, 2013 at 17:00
69

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 !!!

8
  • I really hate working on some old code, where some helper methods are spread throughout the whole codebase. Usually I start putting them in some helper classes, but this is so much work.
    – Simon
    Dec 5, 2011 at 8:34
  • 34
    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, 2011 at 8:51
  • +1 bringing up (bad) memories, but it was the worst maintenance nightmare. Dec 5, 2011 at 10:21
  • The work case of copy and paste I recall had 12 copies of code to handle 6 identified cases. Fortunately, it was a stand-alone module, so it was possible to avoid upgrading it when the rest of the code was upgraded. We couldn't even cleanly parse what it was doing to rewrite it.
    – BillThor
    Dec 5, 2011 at 12:44
  • 3
    "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, 2011 at 13:11
54

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.

Though keep in mind that one should not write more complex, but shorter code. The most maintainable code is simple and simply contains no redundancy to keep it as short as possible. Delegating tasks to other programs or API can also be a viable solution as long as the API calls are simple enough.

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

6
  • 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, 2011 at 10:57
  • I've seen this so many times I've started calling it the "Winchester Mystery House Syndrome" (see en.wikipedia.org/wiki/Winchester_house ). Some code bases I've seen needed a good 'exorcism'.
    – jfrankcarr
    Dec 5, 2011 at 14:48
  • 2
    "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. Dec 5, 2011 at 17:12
  • 3
    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, 2011 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, 2011 at 19:48
35

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).

3
  • 4
    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. Dec 5, 2011 at 15:59
  • @PeterAllenWebb: I'd say that keeping the design simple should generally have a higher priority than preparing it for changes that are likely (rather than certain). That doesn't mean you should never do it. Dec 5, 2011 at 16:18
  • I'd add KISS an especially Travel Light to the related principles list. The latter weighs more than YAGNI, that only wastes time in creation, but former keeps the thing bleeding and even carry a snowball effect.
    – Balog Pal
    Jun 6, 2013 at 4:22
22

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.

18

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.

5
  • 4
    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, 2011 at 15:19
  • You shouldn't have to force it to be testable, and I agree - some is difficult to unit test but that doesn't mean you can't have unit tests around that "hard to test" code that can highlight potential breakages.
    – Deco
    Dec 5, 2011 at 15:40
  • The place where I work, our software was developed without tests. 5-10 years later we're still maintaining and adding features to it. Every time the code is touched, something else stops working correctly. It makes our clients pretty angry. If only we had unit tests... and a few other things.
    – Phil
    Dec 5, 2011 at 16:01
  • 1
    I'm talking about things like UI rendering - unit testing a button renderer would require something like pixel comparisons against an expected result, but running it on a system with a different configuration would trigger a truckload of false negatives. Focusing on unit tests alone gives a false sense of coverage - you need to apply adequate testing at all levels.
    – tdammers
    Dec 5, 2011 at 16:10
  • 1
    the test should be performable in your head. Example: Code says: Line1: a = 1 Line2: a = a + 1 Line3: print (a). Do you honestly need a test to assert a=2? Dec 5, 2011 at 18:02
16

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.
3
  • "oral tradition": yes, but written traditions, e.g. a wiki, is even better. Dec 5, 2011 at 18:04
  • 1
    @SteveMelnikoff - This is ironical: oral tradition on a software project is a barrier against maintainability.
    – mouviciel
    Dec 5, 2011 at 19:08
  • +1: For oral tradition transmitted from one generation of developers/maintainers to the next one.
    – Jim G.
    Dec 5, 2011 at 19:09
5

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=42 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.

2
  • This is known as the "God Object" antipattern.
    – user281377
    Dec 5, 2011 at 12:06
  • yesss now I remembered it Dec 5, 2011 at 12:23
4

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.

2
  • @Jim G.: sorry I stepped on your edit. I put it back in. Dec 5, 2011 at 15:58
  • Most of the maintenance I saw in practice was involved with defects. As project grows older that takes over more, then quite all the time, and new features can't be added. Size of diff in that area is bad measure, a bug may take a 1-5 like fix several people chased for weeks. Also DRY is not problem because number of lines. Deviation hits you not in the time wasted on editing 3 more lines but the weeks on remainder bug for the 2 other instances you missed.
    – Balog Pal
    Jun 6, 2013 at 4:15
2

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.

2

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.

2

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

1

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.

1
  • Isn't this a bit tautological? :)
    – hugomg
    Dec 5, 2011 at 16:25
1

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.

-1
  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.

1
  • 4
    I think you have YAGNI backwards. YAGNI is the principle to not write unused features, not the unused features.
    – Kevin Reid
    Dec 5, 2011 at 17:15
-2

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.

2
  • I would agree except vendors have a habit of going up in smoke, or at least their products do. Dec 5, 2011 at 14:44
  • 1
    -1: have as little code as possible and shove the work to others (i.e. the software vendors) who do the maintenance for you.: No. Please read: codinghorror.com/blog/2008/10/…
    – Jim G.
    Dec 5, 2011 at 15:33
-3

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.

2
  • 1
    Better, but now the gotcha is what makes code easy to understand?
    – Freiheit
    Dec 5, 2011 at 18:18
  • 1
    Your misquote is incorrect - "Make is as simple an possible than make it simpler." : a majority of software engineers are prone to over engineering. Most code I work on could be greatly simplified and still achieve the required results. Further, code that is too simple to do the job is easier to fix than code that is more complex than needed and broken.
    – mattnz
    Dec 5, 2011 at 21:05

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