1

The create and update methods in my model base go through a few steps: pre_save, udpate_database, post_save. The pre-save and post-save are specifically designed to help with object-relational impedance mismatch. The idea is that any database operations that aren't a simple "save value X to column Y in the table" are handled in pre_save or post_save. So for instance you could imagine something like (contrived example for clarity):

$book_data = [
    'isbn'     => '1-2',
    'name'     => 'a cool book',
    'chapters' => [
        [ 'id' => 1, 'A Cool Chapter' ],
        [ 'id' => 2, 'The Coolest Chapter' ]
    ]
];

$book = new book();
$book->create( $book_data );

In this example chapters is data for a child table in a one-to-many relationship. pre_save and post_save are responsible for making sure the information in chapters gets to the proper table, while update_database actually updates the books table with isbn and name. I have found this general organization to be very effective, but I've ran into an occasional problem with it.

pre_save and post_save have their own tasks that are built into the model base and always have to be executed. They are also intended to be extended to add more functionality as needed. So in the above case my post-save might look something like:

class book extends model_base {

    protected function post_save( ... ) {

        // necessary so the model base can do its thing
        parent::post_save( ... );

        // then our custom code
        if ( isset( $input['chapters'] ) ) {
            $this->set_chapters( $input['chapters'] );
        }
    }
}

The problem I have run into is that I occasionally forget to call the parent method, which causes subtle bugs. I usually figure that out pretty quickly. However, the other developers on my team also forget to call the parent, and they tend to figure that out less quickly. So I'm thinking about a small refactor so that there is no need to call a parent. My model base would effectively change to something like this:

class model_base {

    public function create( $data ) {

        // ... normal create stuff here

        // now call $this->do_post_save instead of $this->post_save()
        $this->do_post_save( ... );
    }

    protected function do_post_save( ... ) {

        // ... do the stuff that needs to always happen here

        // then call the old post_save
        $this->post_save( ... );
    }

    protected function post_save( ... ) {
        // this does nothing by default
    }
}

I'm basically adding an additional method that acts as an intermediary between the create and post_save methods. This way there is no longer a need to call the parent method from inside post_save, so no more trouble with forgetting to do that. I'm leaving the current name, usage, and calling sequence of post_save unchanged, so no code should break (of course our tests will be the final measure of that). The only potential issue is that I'm making the model base a little less straight-forward, and anyone who sees these two similarly-named methods might be confused at first glance (I'll put in comments to help for anyone who looks, of course).

Architecturally though, is this additional layer of separation a step forward or backward? Especially because this is the model base, I want to make sure any changes make sense.

2

This pattern is called "template method", and is indeed a common way to solve the problem you describe. It is a step forward if it helps you avoid certain problems, which seem to be the case.

  • No matter how many design patterns you learn there is always one more you've never heard of... or maybe that's just me :) Putting a name to it is a big help, thanks! Any hints on what a better naming convention might be? do_post_save and post_save are not very descriptive. – Conor Mancone Jun 7 '17 at 20:02
  • I usually see it as PostSave and PostSaveCore in C#, but there may be other conventions in php. Not sure if there is a way in php to mark a method as virtual or not, but if so, your do_post_save should be marked as non-virtual - you don't want your subclasses overriding it (else you're back in the same situation)! – mmathis Jun 7 '17 at 20:23
  • No, we don't have virtual/non-virtual methods. That would be perfect though. Instead I just put a clear warning in the docblock for the method. I like PostSave and PostSaveCore. Other than the fact though (as you can see) that I am more of a snake_case person in PHP :) I compromise and go camelCase in Java/Type Script, but I've been doing PHP snake-cased for too long to change! – Conor Mancone Jun 7 '17 at 20:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.