What to do with over-complicated inherited code

Question

Ever since I gained experience at a software house I have found it difficult to tolerate code not neatly structured, without any form of architecture or beauty or clarity whether or not it works i.e. does what its supposed to do.

I find I am always caught up in refactoring code like this sometimes "at my own expense" is where my comfort v productivity dilemma is.

What is the quickest way to deal with badly written code you inherit in your experience? Write yours or encapsulate / refactor what's there, time consumingly?

Answer 1

There's obviously no right answer to your question. In some cases, you refactor. In others, you start from scratch.

Answer 2

Sometimes the best answer is to leave it alone except at the borders, where it interfaces with higher-quality code. Once the interface is clean, you can always clean up the ugliness inside, or just replace it. But without those interface walls in place, any attempt at fixing part of the mess becomes a lot like pulling at a loose thread on a sweater and watching it all unravel.

Answer 3

I heard a podcast on Hanselminutes on this topic just the other day talking with Michael Feathers, who has a book on this: Working Effectively with Legacy Code. I encourage a read. I myself have dealt with a lot of bad legacy code in the past, and generally think that you should refactor/rewrite the bad parts gradually, being careful that you don't cause too much breakage, and be strategic about what to refactor/rewrite. I also encourage the this read from Joel: Things you Should Never Do, Part I

Answer 4

If it works, leave it alone. We're not paid to make code look beautiful, we're paid to make it functional. I'm not advocating writing ugly code, by all means make your own code as beautiful as you want (although beauty is subjective, of course). Just be aware that there is no business benefit in changing code that works, no matter how butt-ugly you think it is.

Show me a customer that cares about the beauty of the code in preference to its functionality and I'll show you a customer that will be out of business in five years.

If there's a bug in that code, you can consider refactoring but, even then, that should be secondary to fixing the bug.

Answer 5

Refactor the low-hanging fruit

Answer 6

Most important rule of thumb: If it is not broken, do not fix it (even if it is ugly).

So unless there is a bug in the code, or you need to update it to add new features, do not touch it.

Second rule: If at all possible make sure you have unit tests in place before you start refactoring. Test cases both for the part that is currently broken, and for the parts that currently work.

Third rule: Don't rewrite/refactor everything at once. Try to break it down into small work units, and make sure that each of them works properly before starting the next.

Also, I find that rewriting and replacing (piece by piece) is generally more productive than refactoring.

Answer 7

Depending on how much time you have and on how crappy the code really is, the scenario ranges from throw it all away and rewrite everything from scratch to leave it as it is and take the pain from the uglyness.

Rewriting everything is most likely not an option, if the code actually works, even though it might be very ugly. However, to expand that code with new features might be impossible without refactoring. Take your time and do small changes. Compile and test often!

If the code is both buggy and ugly and you need to introduce new features as well. Then it might be really worth it to consider rewriting the whole thing. There can only be so much spaghetti in the code, until it becomes unmanagable.

Go with your guts. If it feels bad and messy when you're coding, because the code is ugly and nasty. Just start changing it piece by piece, soon the code "will be yours", you will know what the code actally does, you're making it your territory :)

Cheers !

Answer 8

Beauty is in the eye of the beholder.

Architecture? Are you one of those architecture astronauts?

And BTW your code sucks too. Yes, it really does.

Answer 9

I'm going to assume that when you say you "inherited it", you mean that you have been hired into a position where the existing code base was a mess. The other interpretation is that it was left to you in a will - which would be very cool, but unlikely...

I think that the most important thing you can do is review the code, identify risks and plan to address them. This is a very professional and measured approach to fixing the problems and should be viewed favourably by management. What you need to communicate very clearly is the value you will add to the business though your refactoring efforts given that it doesn't necessarily add features that translates into sales (or whatever the business does) in the short term.

The benefits include things like:

• Bug fix times are reduced, leading to faster turn around of getting patches to clients.
• Feature development time will be reduced while quality will be increased.
• Decoupling components allows them to be tested in isolation, making testing more effective at finding and pinpointing bugs. This leads to increased productivity and quality.
• Overall the software development will cater for change more effectively as a loosely coupled, well tested system can be extended and reconfigured quickly while maintaining a very high quality standard.

You need to find reasons to refactor that have a direct correlation with the business's drivers. If you do not know what dives the business, then have a discussion an appropriate management type who can help.

My advice is quite simple - refactoring is good so long as it's done for a good reason. A reason like "the code is not neatly structured, without any form of architecture or beauty or clarity" is not motivation enough. Engage management and get them excited about the possibilities that an improved code base will give them!

... and then deliver it. If you don't, you're going to look like a clown.

Answer 10

Do what you can, but always remember: it's their code, not yours.