Code refactoring on bad system design

Question

I am a junior software engineer who've been given a task to take over a old system. This system has several problems, based on my preliminary assessment.

1. spaghetti code
2. repetitive code
3. classes with 10k lines and above
4. misuse and over-logging using log4j
5. bad database table design
6. Missing source control -> I have setup Subversion for this
7. Missing documents -> I have no idea of the business rule, except to read the codes

How should I go about it to enhance the quality of the system and resolve such issues? I can think of using static code analysis software to resolve any bad coding practice.

However, it can't detect any bad design issues or problems. How should I go about resolving these issues step by step?

Answer 1

Keep in mind that this legacy system, with all it's spaghetti code, currently works. Don't go changing things just because they don't look as pretty as they should. Focus on stability, new features & familiarity before ripping old code out left right and centre.

Answer 2

My standard answer to this question is: Refactor the Low-hanging Fruit. In this case, I'd be inclined to take one of the 10K-line classes and seek out opportunities to Sprout Class, but that's just my own proclivity; you might be more comfortable changing other things first (setting up source control was an excellent first step!) Test what you can; refactor what can't be tested, take a step at a time, and make it better.

Keep in mind as you progress how much better you are making things; if you concentrate only on how bad things still are, you're likely to become discouraged.

Answer 3

First, don't fix what isn't broken. As long as the system you are to take over works, leave functionality alone.

The system is obviuosly broken when it comes to maintainability, however, so that is what you tackle. As mentioned above, write some tests first, get the source backed up in a cvs, and THEN start by cleaning up small pieces first, then the larger ones and so on. Do NOT attack the bigger architectural issues until you have gained a good understanding of how the system works. Tools won't help you as long as you don't dive into the code yourself, but when you do, they do help a lot.

Remember, nothing is "perfect". Don't over-engineer. Obey the KISS and YAGNI principles.

EDIT: Added direct link to YAGNI article

Answer 4

Try to create some unit tests first that can trigger some actions in your code.

Commit everyting in SVN and TAG it (in case that something goes bad you'll have an escape pod).

Use inCode Eclipse plugin http://www.intooitus.com/inCode.html and look for what refactorings it proposes. Check if the refactorings proposed seem ok for your proble. Try to understand them.

Retest with the units created before.

Now you can use FindBugs and/or PMD to check for other subtle issues.

If everything is oka you might want to check-in again.

I'd also try reading the source in order to detect some cases where patterns can be applied.

Answer 5

First and foremost, make sure you have source control system installed and all source code is versioned and can be built.

Next, you can try writing unit test for core parts of your system. From there, when you have a more or less solid body of regression tests, you can actually proceed with refactoring.

When I encounter messy codebase, I usually start with renaming poorly-named types and methods to better reflect their initial intent. Next you can try splitting huge methods into smaller ones.

Answer 6

Firstly, let me say that Working Effectively with Legacy Code is probably a really good book to read, judging by three answers within a minute of each other.

5. bad database table design

This one, you are probably stuck with. If you try to change an existing database design you are probably committing yourself to redesigning the whole system and writing migration tools for the existing data. Leave well alone.