Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What are the best practices for preventing SQL creep?

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.

like image 906
Daniel Bingham Avatar asked Feb 26 '23 04:02

Daniel Bingham


2 Answers

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.

like image 78
Robert Harvey Avatar answered Mar 06 '23 23:03

Robert Harvey


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.

like image 34
timdev Avatar answered Mar 06 '23 22:03

timdev