Hacker News new | ask | show | jobs
by NDizzle 3808 days ago
Yep, this is the only way to handle these beasts. Don't touch anything. Make a ton of tests. Spend some time validating those tests. Get someone to write off that those tests are sufficient. Start refactoring!
1 comments

For a function with 2,000 lines of code, we have to be honest with ourselves and accept that testing will never be sufficient; if there's 2,000 lines of code, you can bet good money that there's global state manipulation as well.

Making the assumption that anybody is capable of sufficiently testing such functions and subsequently re-writing them will only introduce new bugs and old regressions.

Seems harsh, but I've had to work with a few such monstrosities. Global state galore.

Global state is one of the trickiest problems we are trying to solve at my workplace. Our entire, 10 year old, PHP codebase relies on a single static class (and now some supporting static classes) aptly called "Meta". What does it do? Meta things.

It's basically an abstraction over the entire database layer, that has probably over 30 toggles that denote where and how it should fetch data, given a table name and a function call.

And every database call in the entire project is dependent on it.

Test if you can, but I agree. Thinking unit testing will solve the problem can be incredibly naive especially if the code has lots of global state. Sometimes, just setting up all this state for a test is equivalent to rewriting the application. If the rest of the application is written like this, chances are this function makes a ton of 2k line calls of its own. Unit testing just might make sure you aren't doing something catastrophically dumb.
I've been here too. If possible, try and pull someone in who's familiar with the code to hopefully help you test and simplify it.

2000 lines of code is a massive amount of behaviour to understand. There's a minuscule chance you can infer all that behaviour from reading the code unfortunately.

That's why I mentioned getting a write off on the changes. If several peers, project managers, etc write off that they expect the behavior to be one of these things, covered by these other tests, and it's not, well... Everyone just got educated if and when it breaks!