As programmers, we often take incredible pride in our skills and hold very strong opinions about what is 'good' code and 'bad' code.

At any given point in our careers, we've probably had some legacy system dropped in our laps, and thought 'My god, this code sucks!' because it didn't fit into our notion of what good code should be, despite the fact that it may have well been perfectly functional, maintainable code.

How do you prepare yourself mentally when trying to get your head around another programmer's work?

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.
If this question can be reworded to fit the rules in the help center, please edit the question.

2

wow... this question is really relevant to me right now.
–
WalterJ89Sep 20 '10 at 14:53

14 Answers
14

I can't tell you how many times I've said "Oh, this is totally wrong", rewritten it, and then found out the hard way just why that code was written that way. Usually, it's some non-obvious unwritten/undocumented requirement. At least, that's true in the legacy code I'm currently working on.

+1. Other programs often depend on bugs in the existing code, never mind its odd ways of doing things. Before you go mucking with it, understand it!
–
Alex FeinmanSep 20 '10 at 14:20

I had this problem once. I fixed a bug in our internal libraries, but it turned out that a lot of important code was dependent on that incorrect behavior. Yikes.
–
Jonathan SterlingOct 7 '10 at 17:13

Sometimes writing those tests can be very hard. But then, sometimes you can find a loose thread somewhere, unit test that, and then spread the test infection further. So you might have to refactor-under-test a bunch of stuff before you get to the piece you actually wanted to change.
–
Frank SheararNov 16 '10 at 9:49

5

That assumes your company uses, or even understands, unit tests. Most of the time there are no tests for the code, no documentation and a tight deadline to integrate/fix/modify it so you can't just "start writing unit tests" for it.
–
Wayne MApr 13 '11 at 18:20

2

For legacy code base, it is often easier to start with automated regression tests. Unit tests assume there are isolated units in the code which can be tested independently - you have to be very lucky to find such kind of legacy code.
–
Doc BrownDec 17 '13 at 16:47

Happens all the time for me. I'm constantly looking back on old code and thinking to myself: "Who wrote this crap? Oh yeah.. I did." I think it shows that you're growing as a programmer if you can admit that some code you wrote in the past is bad. If you look back on it and say "Yep, looks good to me", either it's damn good code, or you're not progressing. :P
–
JasarienSep 16 '10 at 16:45

Laugh at it, try hard not to judge it too much, and just get through it. It's not good to be a real code-nazi... There is definitely such a thing as 'good enough' or even 'good enough at the time.' There are many times where something is developed or bandaged to fix a crisis, then never revisited.

If it's really bad, see if you can make a case for re-writing it.. if it's not important, just get in and get out.

Often I find it useful to get a feel for what the original devs thought good was.

Look for patterns and themes to what they did and a lot of times you find that there were reasons for some of the odd decisions in the first place.

Sometimes you find that the original dev was actually bad, but you have an idea of what sort of bad they were selling back then.

Either way, after doing this you should have a better picture of where you could start a re-write or what a quick-fix would look like without having to refactor everything.

Most importantly, don't immediately assume that just because it is ugly it is bad. Nothing makes you look more foolish that to spend time modernizing something only to find out it is less capable than the original.

I always reason that ugly code is code that has seen many debuggings, with many subtleties that aren't apparent with a cursory inspection. If I replace it, or deeply redesign it, I need to make sure that I understand absolutely every aspect of what the code does. If I don't have the time to get to the bottom, I must take an approach of minimal risk, making the smallest possible change to achieve my goals.

Usually I'll make a small fix / change, and propose a feature for later development that would excuse getting to the bottom of things and refactoring the whole thing. Then I do my best to ignore the code until the feature ends up on the roadmap.

When legacy code is more than a couple of years old, it may have been written that way because of limitations in the language or operating systems etc. that existed at the time the code was written. Hey it looks bad now but was it bad then? I try to assume the developer had a reason for what he or she did. That reason may no longer apply but assuming there was one instead of just general incompetence (young programmers will be thinking the same thing about your code in 5 years maybe even less) makes you less angry about it. If it works and there are no problems associated with it, cherish that legacy code, no matter how ugly, as it will let you get on solving more exciting problems.

Unless you're prepared to own the code and any necessary fixes in the future, don't touch it. You'll overcome the tendency to want to fix something when you break something you didn't write because you didn't study it well enough before you dove in, and it takes you 2 days and a fire-drill to get it working again.

Don't get me wrong... there are legitimate reasons to refactor code, but if a business demands that the code works, and you "fix" it without knowing the consequences before jumping in, you're asking for a world of hurt.

Refactoring a little at a time can be useful, but be extra careful about changing any tiny aspect of how the code actually behaves unless you understand why that behavior is there and what it affects. Unfortunately, the code that needs it worst is sometimes the hardest to change without touching the behavior, although you can usually straighten out parts of it, or at least comment it.

I'm working almost exclusively on legacy code these days and I always think "Oh sh%t, what were they thinking?". Then I start writing unit tests for the code and that's the point where I really have to analyze the control flow and the dependencies.

Sometimes it's not possible to easily write unit tests. But while I try to, I get information about the code and will understand why it was written the way it is. Sometimes that will prove that the code is really a mess, sometimes I get to understand the thought process of the original developers and can add useful documentation or rewrite a piece of code when I want to add a new functionality.

For me it helps to think that my code will look the same to myself when I come back to it in 12 months.

With experience comes the judgment to know when code is really bad, and when it is just written in a different style. If it's perfectly functional and maintainable and there is good automated test coverage, then it's not bad and you just need to open your mind. You will probably learn something. Bad code is not functional and maintainable.

Here are some markers of truly bad code:

Large blocks of logic have been duplicated instead of refactored.

Circular dependencies between classes or packages

High coupling; low cohesion

Unused variables, writing to variables that are never read, unreachable code.

Reimplementation of standard library functions, e.g. date formatting.

Unnecessarily complex logic; i.e. 50 lines of code where 10 would do nicely.

No comments describing the purpose of classes or methods.

Misleading comments.

A lack of automated tests doesn't mean the code is bad, but it means the project is bad.

These are not a matter of taste; these practices make program maintenance much more expensive.

How do you prepare yourself?

Accept the fact that it takes a while to be able to successfully work on a new code base. If it's "perfectly maintainable" and there is high test coverage, it takes less time but it still won't happen immediately. If the code is bad, then the first thing I do is warn the stakeholders that it is in bad shape and initial progress will be slow. If they are skeptical, then I back up my claim by showing them a sample of problems in the actual code and explaining how it varies from industry best practice.