In “Programming and Cold Pizza” I talked about the DRY principle and avoiding repeating the same code over and over.   That’s a fundamental principle that should always be part of how you write code.

Here’s another one.

Avoid specific cases.

That is, don’t write five routines to do slightly different things, if you can write one routine that will do them all.

Here’s a real example. This is a method in a larger object that is used to construct a page list. (The details of all that are unimportant, it’s the way of writing and changing the code that’s at issue.) Suppose you have the following method in PHP:

    private function _appendLiner(&$pagelist, $options)
    {
        if(isset($options['Envelope Liner']))
        {
            $sku = $options['Envelope Liner']['sku'];
            if(strpos($sku, ":") > 0)
            {
                $templatename = substr($sku, strpos($sku, ':') + 1);
                if(!isset($pagelist->pages))
                    $pagelist->addChild('pages');
                $newpage = $pagelist->pages->addChild('page');
                $newpage->addChild('name', 'Envelope Liner');
                $newpage->addChild('is_required', 'Yes');
                $fxgnode = $newpage->addChild('fxg');
                $fxgnode->addChild('name', $templatename);
            }
        }
    }

That’s all well and good. Now after some period of time, you need to do the same thing to handle addresses. So you cut and paste that function and change it to do addresses, like so:

    private function _appendAddress(&$pagelist, $options)
    {
        if(isset($options['Return Address']))
        {
            $sku = $options['Return Address']['sku'];
            if(strpos($sku, ":") > 0)
            {
                $templatename = substr($sku, strpos($sku, ':') + 1);
                if(!isset($pagelist->pages))
                    $pagelist->addChild('pages');
                $newpage = $pagelist->pages->addChild('page');
                $newpage->addChild('name', 'Return Address');
                $newpage->addChild('is_required', 'Yes');
                $fxgnode = $newpage->addChild('fxg');
                $fxgnode->addChild('name', $templatename);
            }
        }
    }

Great, right?

Well, no. You’ve just repeated yourself. A better approach is to realize that the majority of what’s happening there is actually generic and has nothing to do with the specific circumstances. Going back to the original code, you refactor and generalize. Extract the general part out to a new method, have it accept a parameter that makes it perform the specific case you need, and then call that new method from your original method. Refactoring in that way preserves the internal call interface and you don’t need to change any other code. Like so:

    // here's the extracted general method
    private function _appendOptionPage($pagename, &$pagelist, $options)
    {
        if(isset($options[$pagename]))
        {
            $sku = $options[$pagename]['sku'];
            if(strpos($sku, ":") > 0)
            {
                // custom liner was chosen, need to add a surface for it.
                $templatename = substr($sku, strpos($sku, ':') + 1);
                if(!isset($pagelist->pages))
                    $pagelist->addChild('pages');
                $newpage = $pagelist->pages->addChild('page');
                $newpage->addChild('name', $pagename);
                $newpage->addChild('is_required', 'Yes');
                $fxgnode = $newpage->addChild('fxg');
                $fxgnode->addChild('name', $templatename);
            }
        }
    }

    // and now the original method can just call that.
    private function _appendLiner(&$pagelist, $options)
    {
        $this->_appendOptionPage('Envelope Liner', $pagelist, $options);
    }

Now you’ve got a general method that can be made specific by passing parameters. And you’ve done it without breaking any existing usages of the code. So now adding the address is easy:

    private function _appendAddress(&$pagelist, $options)
    {
        $this->_appendOptionPage('Return Address', $pagelist, $options);
    }

You can also take it one step further and simplify by doing something like this:

    // change whatever method was calling _appendLiner to _appendOptionPages
    // and remove any call to _appendAddress.

    private function _appendOptionPages(&$pagelist, $options)
    {
        $pages = array( 'Envelope Liner', 'Return Address');
        foreach($pages as $pagename)
            $this->_appendOptionPage($pagename, $pagelist, $options);
        return $pagelist;
    }

Now you’ve got a generic routine, and a generic call to it. The $pages array is used to make it become specific for each case. Now adding another type in the future requires no more effort than adding a name to an array.

Greater flexibility and easier maintenance with less code and less room for bugs. Everybody wins.