23

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?

Tamara Wijsman
  • 8,259
  • 14
  • 58
  • 94
Bryan M.
  • 1,017
  • 9
  • 14
  • 2
    wow... this question is really relevant to me right now. – WalterJ89 Sep 20 '10 at 14:53
  • 1
    If it ain't broke, don't fix it. :-) – richard Jun 23 '11 at 05:16
  • [Things You Should Never Do](http://www.joelonsoftware.com/articles/fog0000000069.html) and [Big Ball Of Mud](http://www.laputan.org/mud/) should be mandatory reading on this topic for all programmers. – Jan Hudec Sep 13 '13 at 07:20
  • possible duplicate of [Working on someone else's code](http://programmers.stackexchange.com/questions/149762/working-on-someone-elses-code) – gnat Dec 17 '13 at 11:43

14 Answers14

31

For any legacy code base, the correct way to prepare yourself mentally for dealing with it is to start by writing unit tests for it.

Whether it sucks or not, you need to first have the confidence to be able to change it without breaking stuff!

Jeff Atwood
  • 6,757
  • 10
  • 45
  • 49
  • 6
    +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 Feinman Sep 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 Sterling Oct 07 '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 Shearar Nov 16 '10 at 09: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 Molina Apr 13 '11 at 18:20
  • +1 agree. See also _Working Effectively with Legacy Code_ by Michael Feathers. – Matthew Rodatus Jun 23 '11 at 10:59
  • 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 Brown Dec 17 '13 at 16:47
30

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.

Frank Shearar
  • 16,643
  • 7
  • 48
  • 84
11

You wait until you've been around long enough to run into your own crappy legacy code. It's a humbling experience and part of the learning process. I long for the time when I knew everything.

I think Fosco had a great point to be able to put it into the context of potential time & functionality restrictions. Sometimes you're forced to get something working.

And last, come to the understand that this is why you have a job.

JeffO
  • 36,816
  • 2
  • 57
  • 124
  • 4
    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 – Jasarien Sep 16 '10 at 16:45
7

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.

Fosco
  • 1,671
  • 14
  • 19
7

Pick your battles. Know the difference between "I wouldn't write it this way" and "this creates a serious maintainence or support challenge"

AShelly
  • 5,793
  • 31
  • 51
4

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.

Bill
  • 8,330
  • 24
  • 52
3

If I have the time I attack it and kill the poorly written code.

It's war.

3

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.

Joeri Sebrechts
  • 12,922
  • 3
  • 29
  • 39
3

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.

HLGEM
  • 28,709
  • 4
  • 67
  • 116
1

In the past when I didn't have the time to piss on someone else's code and beat it into "my" style, I have had to resort to being very task-driven:

What am I trying to add to this code/fix/make work?

Is what I'm doing working toward that goal? If not, stop doing it, and revert to the last time I was making task-oriented changes.

Am I done with this task? If so, stop tinkering with the code, even though it looks like it was written by non-sentient Martian mold.

Alex Feinman
  • 5,792
  • 2
  • 27
  • 48
1

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.

CokoBWare
  • 538
  • 2
  • 8
1

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.

David Thornley
  • 20,238
  • 2
  • 55
  • 82
0

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.

kevin cline
  • 33,608
  • 3
  • 71
  • 142
0

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.

cringe
  • 608
  • 4
  • 12