Edward Yarnold

I have recently been tasked with making some changes to the central ‘library’ of code that an E-Commerce platform is based on.  Essentially, the ‘models’ for this application are all in a central, shared location, and all of the ‘installations’ of the e-commerce platform have their own set of controllers and views that interact with these models.  The changes I am making involve making the deletion of products reversible; so instead of actually deleting products and associated assets, a flag is merely set in the database; is_deleted = 1.  This, at first glance, seems to be quite a simple task; alter the database tables to add the extra column in, and make a change to the central models in Model_Product::deleteProduct and any product ‘getter’ functions, eg Model_Product::getProductsByCategoryId.  However, due to the way that the platform has been developed and individual customisations to the installations of that platform have been made, things aren’t quite so simple.

The idea of the ‘architect’ of this system was that any interaction with databases or storage of data; ie the models; would be shared in the central library.  This would mean that changes such as the one I worked on with is_deleted = 1 would be simple and made in one place, and one place only.  A true implementation of the ‘Don’t Repeat Yourself (DRY)’ principle.

The nature of this platform is that it is exactly that. A platform.  This typically means that no two implementations of the platform are identical, so to speak.  The platform is built upon and ‘extended’ to do what the individual customers want from their online shop.  This sometimes means that custom product ‘getter’ blocks of code are written.  Often, the ‘quick and easy’ approach had been taken with these , and they had simply been added as standalone functions in the individual project’s library, called things such as getAllProductsFlaggedAsSpecial()  This obviously meant that when I implemented my is_deleted changes in the central library, and wasn’t aware of the custom nature of these custom product selectors, the sites that used those custom product selectors continued to display deleted products.  Big oops!

This now means that I will need to go through the individual implementations of the platform, search for any custom product SQL that is being built, and add the AND p.is_deleted = 0 in to them.

This whole problem got my thinking about how I would approach this issue (The need to build custom product getter functions in individual implementations of the platform), and here’s how I think I’d do it.

I think I would ensure that all custom product getter functions were in fact methods in an extension of the main Model class that then called a buildWhereClause method that added any global product SQL (eg AND is_deleted = 0) in, like this:


class Model_Product extends Base_Model {
...
function buildWhereClause($sql) {
return 'WHERE p.is_deleted = 0 AND '.$sql;
}
...
}



class MyShopNameModel_Product extends Model_Product {
function getProductsFlaggedAsSpecial() {
...
$sql = "SELECT * FROM product ".$this->buildWhereClause('is_special = 1');
...
}
}

Essentially, what I learnt here was that the developer to begin with that wrote these ‘quick functions’ saved his or herself half an hour or so, but cost me (2-3 years later) a fair amount of time in debugging.  I think this is evidence that coding things the quick way and implementing a fast solution is often a false economy, and someone later in the life of the software will have to spend the time saved (and possibly more) to work around the disadvantages of the quick approach.

This entry was posted on Wednesday, August 4th, 2010 at 8:37 am and is filed under Best Practice, PHP, Tips, Web Development. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.