[e_i_pi]

Hi all. I'm creating a new framework for my site, and am a fair way in, and have encountered a problem with the way I'm storing my SQL queries.

Instead of embedding the query text in the Models, I've got a separate file for each query, and store these files in a directory called (oddly enough) Queries. If a particular Model requires access to a query, it reads in the appropriate file. This allows reuse of queries across multiple Models. (See the Filesystem.png attachment)

Now, a particular Model method looks something like this:

// Gets the information about the users latest project
final public function GetUsersLatestProject()
{
	$index = 'GetUsersLatestProject';
	$bound_values = array(
		'UserID'	=>	User::ID(),
	);
	return PSFetch::AssocUnique($index, $bound_values);
}

... which is pretty lightweight and easy to read. You may not know what each of my called methods is doing, but the program flow is pretty obvious:

• Set the index
  
• Set the bound values
  
• Fetch a unique associative array from the indexed query, passing the bound values

And for reference, here is the query file for GetUsersLatestProject:

SELECT
	{SQL_PROJECTS}.`ID`
FROM {SQL_PROJECTS}
INNER JOIN {SQL_PROJECT_OWNERS} ON {SQL_PROJECT_OWNERS}.`ProjectID` = {SQL_PROJECTS}.`ID`
WHERE {SQL_PROJECT_OWNERS}.`UserID` = :UserID
AND {SQL_PROJECT_OWNERS}.`IsOwner` = 1
AND {SQL_PROJECTS}.`CreatedAt` = (
	SELECT
		MAX(`CreatedAt`)
	FROM {SQL_PROJECTS}
)

...and to explain this file, anything in curly braces gets preg_replaced via templating, and anything preceded with a : character is a bound value.

So now that I've explained a little, here is the problem:

I'm a fraction of the way through fleshing out the entire first draft of my application, and I'm already up to 29 separate queries. So, the Queries directory is starting to get, let's say, unmanageable. It's an eyesore really. The reason I've stored queries separately is so that multiple Models can access the same query, and therefore any query changes that need to be made due to DB schema changes need only be applied once to affect the entire application.

I'm now leaning towards defining queries within the Models, which I've seen other programmers on this forum do, such as Dormilich. Can anyone give advice as to whether I should be changing my Model system and put the queries inside the Models? The direct fallout of this is that query strings are no longer centralised.

P.S. I've already thought of having parent Models that have the shared queries, the problem is I might have one Model that uses query A and B, another Model that uses A and C, and a third Model that uses B and C. Due to single-class inheritance, I can't comprehensively push all shared query methods up the class tree

Attached image(s)

• 

[Dormilich]

e_i_pi, on 22 December 2011 - 10:59 PM, said:

And for reference, here is the query file for GetUsersLatestProject:

SELECT
	{SQL_PROJECTS}.`ID`
FROM {SQL_PROJECTS}
INNER JOIN {SQL_PROJECT_OWNERS} ON {SQL_PROJECT_OWNERS}.`ProjectID` = {SQL_PROJECTS}.`ID`
WHERE {SQL_PROJECT_OWNERS}.`UserID` = :UserID
AND {SQL_PROJECT_OWNERS}.`IsOwner` = 1
AND {SQL_PROJECTS}.`CreatedAt` = (
	SELECT
		MAX(`CreatedAt`)
	FROM {SQL_PROJECTS}
)

isn’t the last clause equivalent to AND {SQL_PROJECTS}.`CreatedAt` = MAX({SQL_PROJECTS}.`CreatedAt`) ?

e_i_pi, on 22 December 2011 - 10:59 PM, said:

I'm now leaning towards defining queries within the Models, which I've seen other programmers on this forum do, such as Dormilich.

I do?

well, to be honest, up till now I didn’t have to deal with more than half a dozen queries, so it doesn’t matter where I store them. and I normally do not have to re-use them as well.

mostly I delegate the complicated SELECT/UPDATE/DELETE stuff to SQL Views and Triggers.

of course you could structure your query folder as well using a loading mechanism based on the name … (similar to autoloading classes)


PS. thumbs up for using PDO

This post has been edited by Dormilich: 22 December 2011 - 03:24 PM

[e_i_pi]

Dormilich, on 22 December 2011 - 10:21 PM, said:

isn’t the last clause equivalent to AND {SQL_PROJECTS}.`CreatedAt` = MAX({SQL_PROJECTS}.`CreatedAt`) ?

Oh yeah, look, I could probably use HAVING and GROUP BY to get the same result, but I've been a tad lazy there. I generally hack code in then go back and review it after I'm done.

Dormilich said:

e_i_pi, on 22 December 2011 - 10:59 PM, said:

I'm now leaning towards defining queries within the Models, which I've seen other programmers on this forum do, such as Dormilich.

I do?

Yep, I was sure I'd seen it, and it's in your Introduction to PDO tutorial. It stood out, because the syntax you used was syntax I'd not seen before:

$sql = <<<SQL
SELECT 
	COUNT(*) 
FROM 
	`login_table` 
WHERE 
	`username` = :login 
	AND 
	`pwhash` = :pass
SQL;

To me, that's very readable, and a technique I'd be happy to implement inot my code base.

Quote

of course you could structure your query folder as well using a loading mechanism based on the name … (similar to autoloading classes)

That's what I'm leaning towards, but I need to convince myself that it's the right path to take. I'm very reluctant to do file system / schema changes without being thoroughly convinced that it won't need to be redone in 2 months time. I have done something similar with my CSS of all things, and it works very well, even though it all resides in the one file. I guess I could go with a directory structure like this:

• Users
  
  • Get
    
    • UsernameFromId
      
    • UserPreferences
    
    
  • Set
    
    • UserPreferences
  
  
• Projects
  
  • Get
    
    • ProjectTypes
      
    • ProjectModel
    
    
  • Insert
    
    • Project
      
    • ProjectOwner
      
    • ProjectAttachment
    
    
  • Set
    
    • ProjectName

Or, I could push the transaction types up to the root, so that Get/Set/Insert sit at the top, and the Model types sit underneath, such as Get > Project > Types, which follows English language semantics a little better, and compacts filenames without reducing meaning.

Quote

PS. thumbs up for using PDO

Wouldn't have known about it if the DIC gurus hadn't have hammered it into my head for months on end

[AdaHacker]

I think you might want to stop and ask yourself what the goal of separating your SQL like this really is. What can you accomplish by writing your application this way that you couldn't by embedding the SQL in your model classes?

Personally, I've experimented with similar methods a couple of times in the past and quickly ended up abandoning them. In my case, they simply didn't offer any meaningful payoff. That is, it became apparent that the amount of effort it was taking me to separate the queries out far outweighed any maintenance reduction I could expect down the line.

The reason for this is simply that I was trying to solve a non-problem. While separating out your queries to allow reuse sounds good on paper, in my experience it just isn't a real problem. In a well-written application, there generally isn't much direct reuse of queries. Rather, database access will be encapsulated in classes or functions and those are what gets reused. The result was that I ended up with a bunch of theoretically "reusable" queries that were, in fact, only used in one place. So that extra layer of indirection didn't end up buying me anything.

Your example code actually illustrates this pretty well. All your GetUsersLatestProject() method does is call that query and return the results. How is that better that just putting the SQL inline in the method? I mean, when you need that data, aren't you just going to call the method anyway? So what's the point of having the query separate? And if you're going to run the query directly rather than call the method, then why bother writing the method in the first place? It seems to me that all you're really accomplishing is to move the SQL out of the context in which it's used, and it's not clear that you gain anything in terms of readability or maintainability from that alone.

Of course, I'm not saying that there aren't legitimate reasons for doing something like this. I don't know the specifics of your case, so I can't know if this is really appropriate or not. I just think it's important to consider the cost-benefit analysis with a design like this. If you're not careful, it's very easy to get lost in the details and spend a lot of time building an elegant solution to a problem that you don't actually have.

e_i_pi, on 22 December 2011 - 04:59 PM, said:

I've already thought of having parent Models that have the shared queries, the problem is I might have one Model that uses query A and B, another Model that uses A and C, and a third Model that uses B and C. Due to single-class inheritance, I can't comprehensively push all shared query methods up the class tree

You're looking at this part the wrong way. You need to think in terms of decomposition. If you have several unrelated classes that need to access the same data, the solution is simple: abstract that data into a new class. So in this example, you could create one or more new model classes (depending on how things logically fit together) with methods to encapsulate queries A, B, and C and let your original model classes call those. Don't get stuck on inheritance - there's no reason a model can't have a dependency on another on other models. In fact, that's kind of the point.

[e_i_pi]

AdaHacker, on 23 December 2011 - 03:51 PM, said:

Personally, I've experimented with similar methods a couple of times in the past and quickly ended up abandoning them. In my case, they simply didn't offer any meaningful payoff. That is, it became apparent that the amount of effort it was taking me to separate the queries out far outweighed any maintenance reduction I could expect down the line.

Yeah this is a problem I'm finding in various key parts of my application. It's only once you get stuck in to producing the content that you realise some parts of the framework might have too much maintenance overhead for the particular pattern to be worthwhile, no matter how robust it is

Quote

The reason for this is simply that I was trying to solve a non-problem. While separating out your queries to allow reuse sounds good on paper, in my experience it just isn't a real problem. In a well-written application, there generally isn't much direct reuse of queries. Rather, database access will be encapsulated in classes or functions and those are what gets reused. The result was that I ended up with a bunch of theoretically "reusable" queries that were, in fact, only used in one place. So that extra layer of indirection didn't end up buying me anything.

I think this is the paragraph that has really cemented it for me. It really resonated, thanks

Quote

I just think it's important to consider the cost-benefit analysis with a design like this. If you're not careful, it's very easy to get lost in the details and spend a lot of time building an elegant solution to a problem that you don't actually have.

Yeah, it doesn't actually feel elegant you know. If I want to look at what information a model returns, I have to open up the model file AND the sql file. It's a pain in the neck really, and doesn't give any real benefit.

Quote

You're looking at this part the wrong way. You need to think in terms of decomposition. If you have several unrelated classes that need to access the same data, the solution is simple: abstract that data into a new class. So in this example, you could create one or more new model classes (depending on how things logically fit together) with methods to encapsulate queries A, B, and C and let your original model classes call those. Don't get stuck on inheritance - there's no reason a model can't have a dependency on another on other models. In fact, that's kind of the point.

Hmm. Yeah the problem I have is that there are multiple "modules" to my application that all interlace. It's hard to separate concerns as is, and I'm trying to avoid heavy classes and instead go for more numerous lightweight classes. If I were to roll all the pseudo-common stuff up, I'd have extremely heavy base classes, which is what I'm trying to avoid.