2

This is a frequently asked topic... And I have read through many posts, articles and am about to read the book Working Effectively with Legacy Code, but before that and mainly because it will take more than a couple days for book to arrive, and I would like to keep my sanity in check, here's the question:

I am looking at not-that-old (1-2 years) code by a company I recently joined, and I have found so many bad practices in there that it's almost heart breaking...

Massive code duplication, global variables (which are inconsistently used), and things like using a factory class to instantiate various objects that extend a parent object... and then calling methods that don't exist in the parent, but are defined in all the subclasses...

The code works... It's rigorously QAed and bugs are at a minimum, which is fine... but I cringe so much looking at the code...

I recently asked my boss if I could refactor the entire codebase, he asked how I would be able to guarantee the changes will not break anything, I asked we could invest some time to create real unit tests, as their so called unit tests are actually end-to-end tests, so we have to trace the debug log very time something breaks. He said we don't have enough time to create the unit tests (actually the code isn't very unit testable, since there is so much tightly couple code... and I would've had to pretty much re-write most of the classes), so in effect I couldn't even do minor changes to code if it isn't listed as a bug in the issue tracker...

What I'm doing right now is alot of copy and pasting... then fine tuning that since the logic isn't exactly the same...

What can I do? :/

Sorry I think I'm just venting here, and there is nothing I can do in the short/medium term... sigh

Populus
  • 187
  • 8
  • 2
    You've said it yourself "The code works ... rigorously QAd .. bugs at a minimum". I think it'll be very hard to make a business case here. Perhaps you should focus on bringing in better practices for future development instead? – Andy Hunt Feb 26 '14 at 22:46
  • 2
    The age of the code doesn't determine whether it's legacy code. Code written yesterday without unit tests is legacy code today. – kevin cline Feb 26 '14 at 23:29

2 Answers2

6

Do NOT, under any circumstances, change fragile code that you do not have to. While it's fine to proactively add tests so you can refactor bad practices, doing so is at best an as-time-allows activity which should not detract from whatever is is they hired you to do.

If the code is a mammoth ugly behemoth of GOTOs and broken encapsulation and copy-pasted craziness, and it works, then you should leave it alone as much as possible for as long as possible. Your time is better spent on scaffolding a test-driven framework for an eventual replacement, into which you can then migrate and refactor the useful component of said ugly behemoth.

(Not that even doing that is a worthwhile expenditure of time, unless there's some looming Y2K-like deadline that may destroy the behemoth. Or your co-workers are continuing development on said monstrosity.)

DougM
  • 6,361
  • 1
  • 17
  • 34
  • 1
    +1 for the first sentence. If the stakeholders aren't already aware, OP should inform them that changes will become more and more expensive until the code is reorganized, and that the cost of the reorganization will also rise over time as the pile of WTF gets higher and higher. Basically, they can pay now or pay more later. – kevin cline Feb 26 '14 at 23:26
3

There's nothing stopping you from creating "real" unit tests whenever you fix an existing bug or add a new enhancement, and there are some good techniques in Working Effectively with Legacy Code on just how to do that.

If the code's as brittle as it sounds, you're likely going to get plenty of opportunities to fix bugs; just make sure you put good tests in to support your bugfixes and enhancements as they arise. Over time, either you'll eventually grow a substantial test suite or the application will go away (at which point the lack of test coverage is no longer a problem :-)