Refactoring Legacy Code: A New Hope

The Quest Begins (The “Why”)

I still remember the first day I opened the legacy repo at my last job. The README was a single line: “Good luck.” Inside, a 5‑kiloline monster lived—a mix of PHP, JavaScript, and a sprinkle of bash that seemed to have been written during a caffeine‑fueled all‑nighter. Every time I tried to add a tiny feature, I felt like I was stepping on a landmine. Change one line, and somewhere else a silent bug would pop up, usually in a place I hadn’t touched in weeks.

The real villain? Global state. A singleton called Config held everything from database credentials to feature flags, and a logger called Log was imported everywhere with require_once('logger.php');. Functions reached straight into these globals, pulling values, mutating them, and leaving side‑effects that were impossible to trace. Writing a unit test meant bootstrapping the entire application, waiting for the container to spin up, and praying that no test would accidentally flip a feature flag for the whole suite.

I spent three hours debugging a null‑pointer that only appeared when a certain CSV file was uploaded. Turns out, a helper function had cleared a global cache earlier in the request, and later code assumed the cache still held the user’s preferences. I felt like a superhero who’d just lost his cape—frustrated, embarrassed, and desperate for a better way.

The Revelation (The Insight)

The turning point came when I read Working Effectively with Legacy Code by Michael Feathers. He talks about seams—places where you can insert a test or change behavior without editing the code itself. The simplest seam you can create in a tangled codebase is explicit dependency passing: instead of reaching for a global, you hand the needed object in as a parameter.

When you stop hiding dependencies behind singletons or static imports, three magical things happen:

1. Transparency – You can see at a glance what a function needs to do its job.
2. Testability – You can pass in a fake logger, a mock config, or an in‑memory database without bootstrapping the whole app.
3. Safety – Mutating a passed‑in object is now obvious; you won’t accidentally affect another part of the system that also held a reference to the same global.

In short, the best practice that changed how I write code is “Replace global state with explicit dependencies.” It’s not a fancy pattern; it’s a mindset shift that makes every line of code easier to reason about.

Wielding the Power (Code & Examples)

Before: The Global‑Grabber

// config.php – a singleton that lives forever
class Config {
    private static $instance;
    private $settings = [];

    private function __construct() {
        $this->settings = json_decode(file_get_contents(__DIR__.'/../config.json'), true);
    }

    public static function getInstance() {
        if (self::$instance === null) {
            self::$instance = new self();
        }
        return self::$instance;
    }

    public function get($key, $default = null) {
        return $this->settings[$key] ?? $default;
    }
}

// logger.php – another global
class Logger {
    public static function info($msg) {
        error_log("[INFO] ".$msg);
    }
    public static function error($msg) {
        error_log("[ERROR] ".$msg);
    }
}

// Somewhere deep in the legacy codebase
function processOrder($orderId) {
    $config = Config::getInstance();
    $logger = Logger::class; // static call, no instance needed

    $logger::info("Starting processing for order {$orderId}");

    $taxRate = $config->get('tax_rate', 0.0);
    if ($taxRate <= 0) {
        $logger::error("Tax rate missing or invalid!");
        return false;
    }

    // … lots of business logic that also reaches into Config::getInstance()
    // and logs via Logger::error()/info()
    return true;
}

What’s wrong here?

• processOrder hides its dependencies. Anyone reading the function must know about the two globals to understand what it does.
• Testing this function means either letting it touch the real config file (slow, flaky) or using crazy techniques like runkit to replace the singleton’s internal state.
• If another developer later changes the default tax rate in the config for a different feature, this function’s behavior changes silently—no compile‑time warning, no test failure.

After: Explicit Dependency Injection

// config.php – now just a plain data holder
class Config {
    private $settings;

    public function __construct(array $settings) {
        $this->settings = $settings;
    }

    public function get($key, $default = null) {
        return $this->settings[$key] ?? $default;
    }
}

// logger.php – injectable logger
interface Logger {
    public function info(string $msg): void;
    public function error(string $msg): void;
}

class FileLogger implements Logger {
    public function info(string $msg): void {
        error_log("[INFO] ".$msg);
    }
    public function error(string $msg): void {
        error_log("[ERROR] ".$msg);
    }
}

// The refactored function
function processOrder($orderId, Config $config, Logger $logger): bool {
    $logger->info("Starting processing for order {$orderId}");

    $taxRate = $config->get('tax_rate', 0.0);
    if ($taxRate <= 0) {
        $logger->error("Tax rate missing or invalid!");
        return false;
    }

    // … business logic, same as before, but now using $config and $logger
    return true;
}

// Somewhere higher up – the composition root
$rawConfig = json_decode(file_get_contents(__DIR__.'/../config.json'), true);
$config    = new Config($rawConfig);
$logger    = new FileLogger();

// Now we can call the function with full visibility
$result = processOrder(123, $config, $logger);

Why this feels like a power‑up:

• The function signature now shouts, “I need a config and a logger.” No guessing.
• In a test, I can pass a new class implements Logger { public function info($m){} public function error($m){} } and a Config with whatever values I want—no file system, no singleton fuss.
• If I ever want to swap the logger for a NullLogger during a performance benchmark, I change one line in the composition root, not every place that called Logger::error().
• The code becomes a set of tiny, composable pieces—exactly the kind of stuff that makes refactoring safe and fun.

Traps to Avoid (The “Boss Fight” Moments)

1. Partial Injection – You might be tempted to inject only the config and leave the logger as a static call because “it’s just logging.” Don’t. Logging is a side‑effect; if you ever want to suppress logs in a test or capture them for assertion, you’ll regret it.
2. Mutable Shared Objects – Passing a Config that others can mutate still creates hidden coupling. Keep the config immutable after construction (or at least don’t expose setters).
3. Forgetting the Composition Root – Dependency injection only works if you have a place where you wire everything together. In a legacy app, that might be the front controller or a service locator you’re gradually replacing. Start small: inject into one function, then propagate outward.

Why This New Power Matters

Once you start treating dependencies as explicit arguments, the codebase stops feeling like a labyrinth and starts feeling like a set of LEGO bricks you can snap together and pull apart with confidence.

• Bugs become visible – If a function suddenly needs a new piece of data, you’ll see it in the signature and be forced to think about where that data should come from.
• Refactoring turns into a game – You can extract a method, move a class to a new namespace, or swap an implementation without fearing that some far‑away module will break because it was secretly reaching into a global.
• Team velocity rises – New teammates can read a function and instantly understand its contracts. No more “I spent two hours digging through includes to figure out why this was null.”

In short, this practice doesn’t just clean up a single file; it changes the way you think about writing code. You start asking, “What does this piece truly need to do its job?” instead of, “What global can I grab to make this work?”

Your Turn – The Quest Awaits

Find one function in your current project that reaches for a global—maybe a config, a logger, or a service locator. Write down what it actually needs, refactor the function to take those dependencies as parameters, and update its callers. Run your tests (or write a quick one) and feel the difference.

When you’ve done it, drop a comment below with the “before” and “after” snippets—let’s celebrate those small victories together!

Happy refactoring, and may your code be as clear as a fresh‑cut diamond. 🚀