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.
