125

This is probably something everyone has to face during the development sooner or later.

You have an existing code written by someone else, and you have to extend it to work under new requirements.

Sometimes it's simple, but sometimes the modules have medium to high coupling and medium to low cohesion, so the moment you start touching anything, everything breaks. And you don't feel that it's fixed correctly when you get the new and old scenarios working again.

One approach would be to write tests, but in reality, in all cases I've seen, that was pretty much impossible (reliance on GUI, missing specifications, threading, complex dependencies and hierarchies, deadlines, etc).

So everything sort of falls back to good ol' cowboy coding approach. But I refuse to believe there is no other systematic way that would make everything easier.

Does anyone know a better approach, or the name of the methodology that should be used in such cases?

3
  • 1
    Do you have access to the original author(s) of the code? If so, you could try to gain some insight from them.
    – Bernard
    Feb 16, 2012 at 2:04
  • 14
    Check out Michael Feathers' book Working Effectively With Legacy Code: amazon.com/gp/aw/d/0131177052
    – saus
    Feb 16, 2012 at 6:32
  • 3
    Another very good book is Brownfield Application Development in .Net
    – Dr. ABT
    Feb 16, 2012 at 9:52

7 Answers 7

148

First off, it gets a little wearing that everyone on this site thinks anything written by anyone else is rubbish.

Understanding code is difficult, admittedly some poor programming practices make it more difficult, but, for any reasonably complex system understanding the internal structure and idioms used is going to be hard, even if its well written code.

Systems routinely run for more than twenty years. Programming methodologies, best practices, design philosophies and fashions change every couple of years, and, programmers pick up the improved styles at different rates. So what would have been considered a state of the art and excellent example of code in 2007, looks old-fashioned and quirky today. As an exercise I suggest you dig out some code you wrote three years ago, I can almost guarantee you will cringe.

So first of you need to suppress the initial WTF response. Tell yourself that the system has worked well enough and long enough for it to become your problem so there must be something good about it.

Try to get a hang of the original coders style, the idioms used, study the weirder bits of code and see if they fall into a pattern.

If the required changes are small then follow the original coding style, that way someone picking up the code after you only needs to get used to one set idiosyncrasies.

If the required changes are large and the changes are concentrated in a few functions or modules, then, take the opportunity to refactor these modules and clean up the code.

Above all do not re-factor working code which has nothing to do with the immediate change request. It takes too much time, it introduces bugs, and, you may inadvertently stamp on a business rule that has taken years to perfect. Your boss will hate you for being so slow to deliver small changes, and, your users will hate you for crashing a system that ran for years without problems.

14
  • 49
    +1, especially for the last paragraph/sentence. So true...
    – Heinzi
    Feb 16, 2012 at 8:20
  • 11
    "If the required changes are small then follow the original coding style, that way someone picking up the code after you only needs to get used to one set idiosyncrasies. " This is the best advice on this thread. Feb 16, 2012 at 16:18
  • 3
    +1 I would add, do try to get the code under test before you change it.
    – MarkJ
    Feb 16, 2012 at 18:58
  • 4
    @MarkJ -- I agree, getting a decent set of unit tests helps tremendously. This can be more difficult than it sounds as after a series of incremental changes and quick fixes there is probably no accurate function spec other than "do what the old version did". Feb 17, 2012 at 2:03
  • 6
    @MarkJ : And exactly how do you know what every edge case is that needs to be tested to be certain that you have complete coverage by "running it". For a trivial program or one with simple inputs, I accept that the approach might work. For a real program, I have grave doubts.
    – mattnz
    Mar 22, 2012 at 1:53
49

Basically, you have three approaches:

  1. Write from scratch. It may work for small applications, but you can't use it for large codebase.

    Note that for small to medium-size codebase, it's not the worst scenario to avoid at all costs.

    There is a good reason to not rewriting from scratch: an old codebase is generally expected to be tested, to contain tricks which make the code working in some circumstances, etc., and when rewriting everything, you will introduce a bunch of bugs which were already solved.

    This is an invalid argument in cases you're talking about, since you mentioned that the code has a very low quality: not tested, with missing specifications, never refactored, etc. Don't rewrite from scratch rule, on the other hand, must be limited to the high quality, QA-tested code.

  2. Use cowboy coding approach: change everything, then test. This may fail or succeed depending mostly on the context.

    • Example 1: you are modifying the business-critical application. You know that your customer would be angry if you break something in existent code, and this customer is convinced that the actual product not only works well, but is also correctly written (or the customer just doesn't understand what is crappy spaghetti code vs. well-written code).

      Here, you can't use cowboy coding, since you must not introduce new bugs. This means that you need to use one of two other approaches.

    • Example 2: your customer tells you that his experience with the previous developer was a disaster. The product doesn't even work half of the time, and is totally unusable as is. The customer understands that modifying the source code so badly written can introduce even more bugs.

      Here, on the other hand, cowboy coding can be a good solution, if you're sure that it will reduce the overall cost, compared to two other solutions.

  3. Start by refactoring the initial code, adding unit tests (and other testing required in specific cases), documenting it, etc., then add your modifications. This may be a good approach when the code is not too crappy and has some value. For example, if I know that the code was written by my more skilled colleague under time pressure and low budget constraints, I'll certainly use this approach since:

    • There are parts of code which are cleverly done,

    • You'll learn lots of things from the code you refactor.

    • Even under time pressure and low budget constraints, a code by a better skilled developer is still a code of certain level. It means that if there is a trick to make the application work in some circumstances, the skilled developer will still leave you a comment explaining why the following line of code was added.

I don't think there is one perfect solution: compare all three for each project or task, assess their respective cost, and select the best one depending on the circumstances.

In general, follow these two rules:

  • Rule 1: the better skilled are the developers who wrote the code, the more refactoring vs. rewriting from scratch you must use.

  • Rule 2: the larger is the project, the more refactoring vs. rewriting from scratch you must use.

Refactor a large database written by somebody more skilled than you. Rewrite from scratch small pieces of code or code written by unskilled programmers.

10
  • +1 -- but experience is not always "good"... I've seen insanely awesome along with insanely poor code from my superiors :P Feb 16, 2012 at 2:58
  • @Billy ONeal: IMO, a skilled developer under time/money constraints will still write code of different level than an inexperienced programmer. When doing a prototype (i.e. writing code which I will throw away for sure, and which must be at the lowest possible cost), I'll skip comments, style, proper architecture, appropriate patterns, etc., but still do everything I can which is at my level of expertise and doesn't cost additional money and time. But, well, this is a very different aspect which I'll be happy to discuss in a separate question. Feb 16, 2012 at 3:27
  • 11
    when experienced means tenured instead of skilled you are screwed either way! 10 years of doing something as a first year grad does not equal 10 years of skill ( experience )! If you meant skilled then say skilled, experienced means different things to different people, and in a negative way many times.
    – user7519
    Feb 16, 2012 at 3:28
  • 2
    Jarod's point was mine -- experienced means "Has been programming a long time" while skillful means "is a good programmer". There's definitely correlation between those two but some of the worst code I've seen has been written by supposedly experienced people. Feb 16, 2012 at 3:35
  • @Jarrod Roberson: thank you for this explanation, I was unaware of the difference of meaning between those two terms and effectively used them as if they were the same. Feb 16, 2012 at 4:14
42

That's basically my day-to-day. There are cases where writing small tests is just too impractical for the reasons you mentioned. You should always keep big tests though, but covering all bases is often impossible. The best approach I find is to spiral outward.

Instead of rewriting from the start, get a good understanding of elements. If there are long functions, seek to cut them in logical chunks. Decouple/detangle the smaller elements, the least complicated non-circular dependencies. Start a parallel library, which you understand well, and start migrating small functions in generic bits.

Do this, and you will slowly unravel the system, and work towards bigger and bigger chunks of code which will also become more modular.

Document like it's a drug and you're addicted to it. I don't mean to write a novel in your code, but chances are the code is not all too documented, so do it .NET or Doxygen style: what the function does, input, output (and if need be what global variables or properties you're using or changing).

Last trick: define default behaviour. If there are options or parameters to the program as a whole, define what is the default, implement an easy way to deploy with all defaults (I did it with a simple INI file. Very useful) explicitly laid out. Then you can add in other options and functionality as new options that tie in to the original system.

2
  • +1 for a better and more concise answer than mine. Also a parallel library is a good idea; it helped me in some projects when I had to refactor legacy code. Feb 16, 2012 at 4:08
  • 2
    You're actually more dead on with several valid approaches. All I have are pointers and tips...
    – MPelletier
    Feb 16, 2012 at 4:44
18

When working with Big Ball Of Mud, the only effective approach to me was to get (more) testers and arrange a thorough professional quality assurance in the project.

  • Thing is, all the approaches applicable with better codebase just don't cut it.
    With good code, I study documentation to learn about the principles, read through code (and especially through unit tests) to figure design details, write my own unit tests to cover the changes I do etc. When code is improved, unit tests remain intact or go through minor changes, docs do not need any major rework etc etc.
     
    With bad code, things seem to work exactly opposite way. Information I get from documentation is mostly obscure and outdated waste. Reading through code just melts my brain with convoluted control flow and counter-intuitive patches applied over earlier patches applied over even earlier patches made long time ago to quickly work around design errors.
     
    As for unit tests, these do things opposite to how I use them in good code, like breaking at reasonable changes and failing to catch the real mistakes I make. Which is painful but not surprising - what else would one expect from testing units which have bad design to start with? Instead of helping you improve the design, unit tests often work to preserve bad code - eg in my recent maintenance project I was regularly removing large chunks of code which was only referenced from outdated senseless unit tests. BTW I use that knowledge when writing new code: when I find out that my unit tests tend to get too complicated / fragile, this indicates a need to fix some issue with my own design.

Now, back to what worked for me. First thing is not to let management trick you with you're good developer, you can handle that rhetoric. Think for yourself, think critically and you'll find out that despite good-developer smoke screens, the whole thing is heavily tilted to quality assurance side.

  • Start by asking yourself - why didn't they just throw away this Big Ball of Mud?
    Why did they invest into hiring you to maintain it? Typically, the reason is - the thing works well for the users and you are expected to keep it work well for the users. They perceive it as a black box, they don't look into the (crappy) code, all they see is a useful functionality.
     
    Now, if you ever worked with professional QA, you'll quickly recognize that it's just the way how testers tend to deal with software. Black box, functionality, quality from user perspective - these are all the quality assurance topics and themes.

Okay, if you've read so far you may wonder how exactly that works for me? It is simple really.

First thing is - whenever I finish some feature of bugfix, I just reassign respective item in issue tracker to QA guy to verify my changes. This way, I don't need to waste time on cumbersome functional testing, just make a quick check that candidate code is ready for QA.

If I am really lucky (doesn't happen often), I don't need to worry anymore. As for the worst case scenario, it's not as bad, either. If I did something wrong, tester gets back to me just a day or two later, with clear explanation of what exactly went wrong and how to reproduce the bug, along with their regression test suite extended to easily catch this kind of bugs if they occur again. Not bad really, don't you think?

Next great thing that comes with QA is regular regression test cycles.

  • Some (lame) managers may try to convince you that you can get this done automagically - don't trust them on that.
    Things that work great with good code, just fail to do the trick with Big Ball Of Mud.
     
    Test execution is cumbersome, analyzing test results is effort consuming, maintaining issues and regressions database requires much focus (more focus than you can afford if you plan to also focus on development). Instead of wasting time trying to be jack of all trades, let professional testers do that for you, free your brain and time for design and coding. Just leverage good old division of labour.

Regular test cycles (imNSho weekly are best, although monthly did work well to me, too) let you do the impossible - effective refactoring.

By effective I mean, if the thing needs say one week for coding and one week for fixing regression bugs, you can expect that you'll spend just that - two weeks as you planned - without sinking into extra month wasted on brain-damaging tests execution and complicated failures analysis - because testers will cover that for you.


Besides main benefits mentioned above, there are other, smaller but pleasant bonuses along the way.

  1. With testers, you get someone to discuss design with. As I wrote, docs and code don't help here when you deal with Big Ball of Mud - making kind of vacuum if you work alone. A tester who runs your code and looks at it from different perspective makes a great partner to bounce ideas and explore possible changes.

  2. With testers, you get someone to backup your understanding of design issues. When there are only developers complaining about code quality, this often sounds like subjective WTFs from behind the closed door.
     
    But when this is echoed by QA guy saying something like component A had 100 regression bugs for 10 new features, as opposed to component B which had 10 regression bugs per 20 new features, communication suddenly turns into whole another game.

  3. The last but not the least, professional QA helps to promote your understanding of amount of efforts worth investing into into design improvements. As I already mentioned, management doesn't grok code quality WTFs very well.
     
    But when there is professional QA, with all the data they usually collect, you can come up with stuff that somehow tends to skyrocket through managers brains right into that secret cell with magic stamp I approve

    During last year, we wasted about 6 man-months solely on fixing regression bugs in the product. Now, what about giving dev team a week or two to analyze if there's something we can to to get this waste cut by half?

3
  • 2
    -1: Huh?? Your answer was long and it left me confused. It seems like you're recommending that the OP lean heavily on QA. // Could you please revise and shorten?
    – Jim G.
    Mar 21, 2012 at 3:35
  • 2
    @JimG. it looks like you understood my answer correctly - I am recommending that the OP lean heavily on QA. Given that there seem to be nothing to revise.
    – gnat
    Mar 21, 2012 at 7:09
  • 1
    This is an excellent answer. Obviously it isn't something you can do alone by yourself, but I know this works well with some real-world projects. It may not be the solution for every situation, but then what is? Nov 23, 2015 at 11:42
10

One approach would be to write tests,

I believe that you sort of half answered your own question. There are a number of ways that you could look at this, however with experience I've found that it always seems to gravitate towards applying some of the standard Agile practices, and do everything in a systematic manner in the same way that you would approach any difficult to solve technical problem.

This is the way I personally approach this sort of situation:

  • Start with a period of study, take notes, draw diagrams, and ask yourself (and/or a nearby 'local expert') lots of questions. You need to read the code, and get a nice big picture overview of the problem at hand.
  • Identify several obvious problem areas and assess the risk of messing with them. Spike a few changes to see how the failures will occur and get an idea about how much additional effort would be required to either follow the path your spike was following, or to do something else entirely.
  • Pick a problem, and write tests. Use existing tests if they are available, and if not write new ones that protect the business logic as much as you possibly can.
    • Write your test to fail, then fix your test to pass, then change your code to fail as a double check, then restore your code to pass again. Use the last "code failure" step as an opportunity to step through the code and identify anything obvious you may have otherwise missed.
  • Design and Plan your change for how you believe the code should be written to fix the problem. This is where you need to go back to the requirements and the spec (assuming you have one), and write your tests as you would your user stories or features, or whatever method you would usually use.
  • Write new tests to match your new stories (aka your revised spec)
  • Refactor your code in little steps. Start by extracting methods, then look to extracting classes, then look to moving methods and classes, and then combining/deleting classes. Always do all of the easy refactorings first, because this will make the more difficult refactorings easier later on. I'd suggest reading both Refactoring and Refactoring To Patterns if you are looking for a little help or inspiration to guide your efforts.
    • As you refactor, you are aiming to keep your original tests passing, and aiming towards having your new tests passing once the refactoring is complete. As you do this, you may find yourself throwing out some of the original tests, or you may be lucky enough to have both old and new tests passing together.

This is a process that I have found works well for me. It is time-consuming, and it can sometimes be tedious, but it works and always gives me a good result. I have never found code that I couldn't improve given the time and resources to do so. Tthe thing is, you may find that during your initial investigations and spikes that you identify some very difficult problems or even show-stoppers based on the time and effort you estimate it all taking, and estimating this sort of thing can be very difficult to do with any certainty. You may find that in the end it will be better to start over, or you may find that the code is of great commercial value to the company you work for. Give your best estimate to get the work done, and if you have a full understanding of the problem domain, give your best estimate to start things over. Present a few cases and scenarios to your boss, and then let management decide where to spend the money. That takes the heat off you and puts it firmly back with them.

As I said, this approach works for me, it might not work exactly the same for you. You may wish to tweak the process, or look at solving the problem another way. The point is that you can only solve these difficult problems by thinking laterally and not seeing a mess in code as simply a lost cause. Think of it instead as a difficult problem and a challenge, and with any problem, there is always a systematic way to solve it. You just need to find an approach that works for you. Personally, when I rise to such a challenge and once I have a few wins under my belt, I take a great deal of pride in turning a poorly designed application into a beautifully crafted product.

4

I have been working on a complex code base for more than a year now. See if my insights can help you:

Your insights are right, by the time you reach a different part of the code, you forget about the previous part. It can be a never ending cycle. The important lesson to take away here is that the product cannot work without all the parts working properly. Even, if one part fails, the product doesn't work. See it from another angle: if you improve one part dramatically, it still MIGHT NOT result in better working of the product, which is your main goal here.

So, at First: Don't be a developer. Be a tester.

Don't try to understand part by part. Understand the whole product and its working when all parts are together. From a production environment(i.e., a non development environment - no debug points), test the product. Then, just like every tester does, log the problems you face into a bug tracker. Assign the severity and priority to it. As this software existed for quite some time, see if there is already a bug tracker created. If there is one already, you are lucky. Add to those and take time and verify each of the existing ones. At the end of this cycle, you understand the product from a user point of view(you definitely shouldn't miss it) and also a QA point of view. Due course, you might even realize that a line of code will fix the bug, and those who coded it didn't do so as there was no real need back then.

Second Step: Wear you designer cape

Break the product into several parts(not literally or according to your convenience, but according to how they work together). May be your work uptil now or existing knowledge might come into play. Then, try to understand how they work with each other as well as with the 10 dependent libraries. Then, for each tracked bug, write your notes identifying entities of code(e.g.: This change involves modifying classes X,Y,Z, etc.). Probably, by the end of this step, you will have FEW hints of what are the problems with current architecture and what can be improved.

Then, you can decide if the current architecture/design is sufficient and you can go with improving the software OR if the product needs a better design or changes in the existing design.

House of Cards

Also, since complex products come with a lot of code, we might not be in a position to pick up a few things and tweak or improve them. This is because the whole system can be intertwined in such a way that making change to one of the classes is equivalent to changing the position of one card in a house of cards, you never know which end might break. In my experience, this has been true. I have picked a part, improved its code, unaware of the contracts it had with other parts of the code and ended up abandoning the code and realizing my mistake. So, instead of trying to understand parts, try and understand it is a whole.

Prioritize your concerns

You need to keep in mind what you are trying to improve:

Do you want the product to be faster?

Of course you do. But is it the primary of the concerns? Is it being slow? If yes, create performance criteria, identify bottlenecks and improve on those parts. Test again.

Do you want to improve the usability?

Then it's pretty much the API/UI side.

Do you want to improve the security?

Then it's the boundaries you should be exploring.

I have provided only 3 examples, but there are a lot more to look for.

Latest and Best Documentation

I have read here in one of the posts that the latest and best documentation is the code itself. Even if you create a good amount of documentation today, it is history after a while. So, code is your latest piece of documentation. So, whenever you browse through some code, write your understanding in the comments there. While passing the code base, caution them to depend NOT ONLY on comments!

1

Write as much documentation as necessary to explain the current code. Once you're sure you understand the logic of it, then you may be ready to change it. Document whatever you're going to add and make sure that you cover all cases.

Write regression tests. It doesn't matter if you manually test or run the tests automatically, you need to preserve the original functionality and regression tests help with that.

If there's not enough specs, start bugging people for more information and write it down.

Write up a plan for changing the code and break it out into different phases.

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