What are the best practices for preventing SQL creep?

Question

I have a webapp written in PHP using a MySQL database backend. This question could just as easily apply to any language and application trying to use an SQL database and MVC OOP design.

How do you keep your SQL code restricted to the Model?

There's a rather longer story specific to my case behind the question. As previously mentioned I'm working on a PHP/MySQL/AJAX website. I designed it using OOP and MVC design principles - with a Model, View and Controller. I managed to keep the view elements - such as markup and styling - restricted completely to the View and to make them reusable fairly easily. I thought I had done the same with SQL code. But as work has progressed, it's become pretty clear that the Model needs some serious refactoring.

The way I'd found to keep the SQL in the Model was to encapsulate every single SQL query in its own Query object. And then when I needed to call some SQL in the View or Controller I would access the query through a factory. No SQL code exists in the Controller or the View.

But this has become extraordinarily tedious. I don't think I'm actually gaining anything by doing this and am spending far too much time creating queries with names like "SelectIdsFromTableWhereUser". The factory for the queries is approaching thousands of lines. A bit of searching in Eclipse has revealed that the vast majority of these queries are used in one or two places and never again. Not good.

I know that in good MVC you want to keep the SQL completely separate from the Controller or the View. But at this point, it seems to me like it would have been better to just place the SQL where it was needed in the code and not bother trying to bury it and database code deep in the Model. These queries are only being used once, why bother encapsulating them?

Is it that important to keep the SQL separate from Controller or the View? What is gained by doing this? What is lost by allowing it to spread? How do you solve this problem?

Edit Per request, here's a little more detail on my model.

There are two portions to it. The Tables portion and the Queries portion. The Tables portion holds the domain objects - mostly designed as wrappers around class objects that are exact analogues of the tables in the database. For example, there might be a database table Foo that has fields id, name, and type. There will be a Table object (class FooTable) that has an array with fields 'id', 'name' and 'type'. It would look like this:

class FooTable extends MySQLTable {
    private $id;
    private $data;
    private $statements;

    public function __construct($id, $data=NULL, $populate=false) {
         // Initialize the table with prepared statements to populate, update and insert.  Also,
         // initialize it with any data passed in from the $data object.
    }

    public function get($field) {}
    public function set($field, $value) {}
    public function populate() {}
    public function update() {}
    public function insert() {}
}

If there's a fooBar database table that has a one to many relation (one Foo many Bars) with fields id, fooID, and bar then there will be a FooBar Table object (class FooBarTable) which will look pretty much exactly the same as the above FooTable.

The FooTable and many FooBarTable objects will both be contained in the Foo object. Give the Foo object factory an id to a Foo table and it populates itself with Foo's data and all of its Bars and their data.

The Query objects are used for pulling out those Foo ids in the order they are needed. So, if I want the Foo objects ordered by date, vote or name, I need a different query object to do it. Or if I want to select all Foo objects that have a Bar in a certain range. I need a query object.

Most of the time I use the Table objects (the wrappers, not the base tables) to interact with the database. But when it comes to selecting which table objects, that's where the queries come in.

During original design I didn't think there would be too many queries and I thought they were going to be things that would see reuse. Since there might be several places where I'd want the Foos in date order. But it hasn't worked out that way. There are way more of them than anticipated and most of them are one offs, used once in some View or Command and then never again. I also thought the queries might encapsulate fairly complex SQL and it would be good to have them as objects so that I would always be sure to give them the data they needed and it would be a relatively sanitized environment in which to test the SQL query itself. But again, it didn't work out that way. Most of them contain pretty simple SQL.

Answer 1

Why not use repositories? Seems to me like this would be a nice, simple way to encapsulate the SQL. Your current approach seems unnecessarily complicated.

The NerdDinner Tutorial has a good example of repository usage in an MVC context; even though it is not in PHP, hopefully it will give you an idea of how this pattern works.

Answer 2

It's impossible to give good advice without knowing what kinds of things you're doing, but it's clear that something is probably very wrong.

From what you're said, it sounds like you're right in thinking that your model needs some major refactoring. In fact, it sounds like it needs some serious redesign (meaning the API your controllers and views use to access it will change).

Some thoughts:

You say:

The way I'd found to keep the SQL in the Model was to encapsulate every single SQL query in its own Query object. And then when I needed to call some SQL in the View or Controller I would access the query through a factory. No SQL code exists in the Controller or the View.Controller or the View.

This makes me think you're missing the point. You model should be more than a bunch of boilerplate code so that SQL doesn't (gasp!) show up in some controller. Your model ought to be a model of your domain object -- what you've described is just an inefficient proxy to SQL.

It might be helpful if you posted some examples of how your model gets used. That is to say, edit your question to include some examples of how your controllers and views use your models.

Answer 3

To start with the last question : what is gained is separation of concerns or in plain english "Keeping things together which belong together". A key word here is belong, which is a rather subjective word.

In the early PHP days a lot of people found that whatever is on a single page "belongs" together. Since PHP used to be an acronym for "Personal Home Page" its design goal was small sites with a couple of pages and then this sense of belonging makes absolute sense.

When things grow, the sense of belonging shifts. When we get a complex data model which needs to be kept consistent and which must evolve over time, then suddenly the operations on this "model" start to "belong" together because it becomes too difficult to dig out the operations and SQL queries all over the place. So in order to keep control we need all model operations on the same page.

As a practical approach I like to draw a line in the sand between the UI and the Model and define a API there which encapsulates the mini user goals (User wants to see the promotions --> getPromotions methods is needed, User wants to add something to the cart : addToCart method is needed, etc...).

I like to draw the line here, because it captures the User's desires, the UI's responsibility is to make the user get there in an easy and effective way, the service/model/repository layer responsibility is to realise that desire.

If done properly, it is also on a level 1 step removed from the User. Many developers however confuse what the User wants with the features that need to be implemented. Then you get very ineffective service methods like saveProject (A User does not want the project to be saved, she just wants it to be there next time she logs in. It is taken for granted, and as such it has no place in the service API). This implementation based API leads to layers of almost empty wrapper methods.

Things like repositories etc, are a way to structure this service layer.

This user-goal oriented API approach also has the tendency to clean up the views (the content of the display elements can be fetched with a handful service calls) and the controllers (an action then consists of the normal sanitizing and typically a single method call).