3

Many web application projects I've been involved with reach a point where

  1. The application expects persisted data to be in a particular format

  2. The application will barf if the persisted data strays from that format

  3. Old "mystery code" is persisting data in the bad format

This usually results in the application developers cluttering the model code with lots of validation conditionals. That is

function save()
{
    if($model->getSomeProp() == 'bad value')
    {
        $model->setSomeProp('good default value');
    }

    return parent::save();
}        

Are there better patterns and/or systems for dealing with these situations, with said patterns and/or systems not relying on having the developers writing perfect migration scripts and/or validation code for every release? My specific interest is in how other developers approach cleaning up these sorts of (in my experience) common long-term problems.

Specifically looking for a LAMP Stack/PHP solution, but solutions and approaches from other common middleware languages/platforms (ruby, python, etc.) are more than welcome.

Alana Storm
  • 164,128
  • 91
  • 395
  • 599

2 Answers2

2

We use configuration-file provided list of steps that should be performed on every processed item. That enables validation, slight changes of data, lookups, retrieval and merging of certain attributes from external sources etc.

Although right now it is based on a set of Ruby classes that implement abstract Step, working according to yaml configuration, I guess that in the next rewrite I would go with pure Ruby DSL.

So at the end, you would have something like this:

HealingProcessor.on(impure_data) {
  replace_bad_value :field => :some_prop, :bad_value => 'bad value', :good_value => 'good_default_value'
  # etc
}
Mladen Jablanović
  • 43,461
  • 10
  • 90
  • 113
2

At least you are handling this type of behavior as close to the db interaction as possible, it could be a lot worse if your code-base were littered with these types of checks.

If I were tasked with cleaning this type of thing up, I think the first thing I would do is set the method to throw a custom exception code, that way I can log the calling code and find which part of my application is formatting data in an incorrect fashion.

For example you could do something like:

class CustomException extends Exception
{
    const CODE_BAD_FORMAT = 1;

    protected code;

    public function setCode($code)
    {
        $this->code = $code;
    }

    public function getCode()
    {
        return $this->code;
    }
}

class Model extends ParentModel
{
    function save()
    {
        if ($model->getSomeProp() == 'bad value') {
            $badValueFound = true;
            $model->setSomeProp('good default value');
        }

        // Now that you are using try/catches you don't need a return value
        parent::save();

        if ($badValueFound) {

            $e = new CustomException();
            $e->setCode(CustomException::CODE_BAD_FORMAT);

            throw $e;
        }
    }
}

// Calling code
try {
    $model = new Model();

    $model->setSomeProp('ohnoes im bad format');

    $model->save();

} catch (Exception $e) {

    if ($e->getCode() === CustomException::CODE_BAD_FORMAT) {
        error_log(__METHOD__ . ': Called save with bad format'); 
    } else {
        throw $e; // Some other exception occurred b/c the code() didn't line up so bubble up
    }
}

// All is well b/c made it through the try / catch block... so onto the next logic

Now, you can make the call to save(), and if a bad format is encountered, you can throw an exception and check the code from the call, if the code matches (expected bad format) then you can implement some logging track calling code-points.

Plus, you don't break anything in the process, because the save is still going to happen, however you will have to ensure any calls to save() are wrapped in a try/catch block, otherwise you will get exceptions if not caught properly.

Another idea might be track the bad format constructs in the model classes so you don't end up copying the same strings all over the place:

class Model
{
    const BAD_FORMAT_MALFORMED_NAME = 'format for a malformed name';
    const BAD_FORMAT_MALFORMED_ADDRESS = 'format for malformed address';
}

....
if($model->getSomeProp() === self::BAD_FORMAT_MALFORMED_NAME) {
....
Mike Purcell
  • 19,847
  • 10
  • 52
  • 89