Refactoring your legacy code - Part Two: Global Problems ?

Today we want to address a global problem. No - not the global warming up - instead the misuse of global scope in variables and functions. This is a followup to my previous posting. And again (as every posting in this series) some advertisement for our talk at the phpconference.

Legacy Applications are often done in PHP4. Which means no really object orientation. All class variables are public - and this is how they are used. Over the time many members are set/get in different places.
If you start writing a test - you need to make sure that you only test specific functions. All subsequent calls, data etc. need to be stubbed == faked.

Quite common would be that you have to deal with many global variables. In OOP applications it shouldn’t  be that the output of the function is influenced by global parameters. And for correct testing you need to refactor this.

Both cases need to be addressed by you - and this is where the refactoring starts. As I wrote in the last posting - while writing the test you have to refactor otherwise you can’t create proper tests.

But what to do with PHP super globals $_GET, $_POST, $_SERVER, $_COOKIE etc.? They are globals by PHP nature. You could create a wrapper (proxy) myInput::getParameter(‘getParam’) and use it everywhere in the code. No exceptions allowed. Doing so you can stub these calls also. And furthermore it adds a bit more security as you can add in this function the very basic and idiotic security checks. They won’t be enough for sure - but it’s a start.

The same applies to the global configuration parameter. The most important here is – you must be able to supply your own parameter in tests here. This way you can write tests for different request parameters and config options. On test tearDown() all parameters should be restored to defaults.

Let’s see how this could look like. You might find such settings in your code:

class someOtherClass {
    var $setting;
 
    function calculateSomething($a, $b) {
        return $a+$b;
    }
}
 
class myOldNastyClass {
 
    function needToTestThisFunction() {
 
        $class = new someOtherClass();
 
        $z = $_GET['input'];
 
        // ....
 
        return $class->calculateSomething( $class->setting, $z);
    }
}

This code has more or less all the pitfalls you will explore while dealing with globals. Before you can even think of writing a test for this you need to refactor it. And this could lead to:

class someOtherClass {
    private $setting;
 
    public function calculateSomething($a, $b) {
        return $a+$b;
    }
 
    public function setSetting($set) {
        $this->setting = $set;
    }
 
    public function getSetting() {
        return $this->setting;
    }
}
 
class myInput {
    public function getParameter($name) {
        return $_GET[$name];
    }
}
 
class myOldNastyClass {
 
    private $input; // set e.g. in constructor
 
    public function needToTestThisFunction(someOtherClass &$class, $z) {
 
        $z = $input->getParameter('input');
        // ....
 
        return $class->calculateSomething( $class->getSetting(), $z);
    }
}

Now we have some code we can test. Let’s see how the tests could be:

class myStub extends someOtherClass {
 
    public function getSetting() {
 
        // fixture
        return 99;
    }
}
 
class myInputStub extends myInput {
     public function getParameter($name) {
        // fixture
        return 999;
    }
}
 
// in the test
 
$test = myOldNastyClass->needToTestThisFunction( new myStub(), 88);
// ....

I hope I could point you in the correct direction ;)

October 16, 2008 | Filed Under Agile Development, PHP related 

Comments

2 Responses to “Refactoring your legacy code - Part Two: Global Problems ?”

  1. Malvolio on October 16th, 2008 9:51 pm

    To boil it down — you can’t refactor until you have tests, and you can’t write tests until you refactor.

    What am I missing?

  2. dodger on October 17th, 2008 8:26 am

    Correct. That’s no secret. The idea of my postings is to show the correct way to do both in one step.

Leave a Reply