Tuesday, 20 September 2011

When Inheriting a Codebase, there are more questions than answers...

There will be that time in your life when you inherit someone else's source tree or codebase and you’ll need to think of a plan to deal with this situation. Now, there are codebases and there are codebases, none are ever perfect, but some are just unbelievable...

The idea of this blog is to go through some of the question’s you’ll need to ask in order to figure out what you can do and whether or not you’ll be able to make a difference or just end up picking away at the edges going nowhere.

So, what sort of questions should you ask? The first thing to find out is what does the customer want to achieve? What kind of future do they envisage? Where do they want to go? After all it’s usually their code you’re working on.

Assuming that your customer wants you to take their codebase forward, you need to consider how you’re going to achieve this. Will it be by Revolution or Evolution?

Revolution or the big re-write approach always seems to fail. At best you re-write your application into that killer-app with all the new features, but in re-writing the basic functionality you’ll lose ground to your competitors. At worst, you’ll end up repeating the mistakes that were made the first time around and your codebase will still degenerate into a same kind of mess. I always prefer the evolution approach, fixing various sections of the code a bit at a time whilst keeping the overall application working.

You should also consider how the application is built. Does it adhere to the 10 minute build rule? Is there a CI machine? Does it use Maven or Ant or something else entirely? If you’re not using Maven then I suggest trying to get that accepted as the way of doing things though actually implementing Maven may be a lot easier than getting approval for its use. Politics eh...

Is the Codebase modularised or amorphous mass?

Try to figure out whether or not anyone thought about splitting the codebase into distinct modules. Is it nicely layered or is everything stuffed into the default package? If it’s all bundled up together then try splitting out different modules - one at a time. Remember that Maven favours smaller modules than Ant. When splitting out modules choose whether to work on layer based (eg. DAOs and Business Logic) or functionality based (eg: ‘book an appointment’) boundaries.

It this a dish of spaghetti code?

If the code resembles a heap of spaghetti it’s not the end of the world, because you can ask the next question:

Does the code have any unit tests?

If it doesn’t have any unit tests (and this isn’t uncommon) then start writing some. The more tests you have the better. When appropriate, take each class and start creating some unit tests. This will give you the confidence to refactor the code, applying the usual SOLID principles. Start with the single responsibility principle and work from there applying it at a method and class level retesting each time to keep those tests passing. When using unit tests, the spaghetti code should soon unravel.

If it has tests, are they any good?

There are tests and there are tests. Figure out if the tests actually mean anything. Do they relate back to any scenarios or requirements? I’ve seen tests that simply said:

String result = testInstance.doSomething();
    assertNotNull(result);

...and where result is a complex XML string. In this case the test is pretty meaningless as you need to verify that the XML string is the XML string you’re expecting, which usually means a lot of parsing and checking. So, if the tests are useless, then write some more.

Did the previous owners follow good or bad practices?

One of the worst practises you see is good old cut ‘n’ paste programming. Maybe a bug as arisen in one class or JSP and someone come along and found a bit of code that fixes it somewhere else in the codebase and copied it into the broken class. When this happens, the codebase has grown needlessly, becoming a more of a mess, and when it comes to fix a bug in the pasted code then you’ll fix it two, three, four etc times. THERE IS NO NEED FOR THIS. As the old sayings go: “Don’t Repeat Yourself” or “Duplication is Evil”... If you’re ever in this situation the very least you can do is to create a simple static helper class and re-use its code.

Inheriting source code should be treated as an opportunity to apply the tools in your toolbox whether they be Test Driven Development, Single Responsibility Principal, continuous integration, dependency injection or whatever, but having said that, ultimately, you need to find out just what your client wants you to do. Are they confident enough to allow the code to evolve and improve, or, perhaps because other programmers haven’t added any unit tests, are they afraid to change something in case you break it? If they're afraid of change then all you’ll do is just pick away at the edges and not get to really make a difference. At this point I really should talk about cultivating client trust, working together, professional relationships and communications, but always remember that in the back of you mind your should always know when to walk away...

No comments: