views:

510

answers:

7

I'm building an object to search orders in my database. There are a bunch of possible parameters that the user can set, and they can set as many as then want for each search. I've created setter methods to collect all the parameters needed for the search.

My question is this. What would be "best practice"

  1. Storing the parameters, and building the WHERE clause when the doSearch method is called
  2. Building the WHERE clause as parameters are set

I'd like to understand the reason behind any recommendation.

Note that the object is instatiated for each search, so I don't have to worry about a second search with different parameters.

+1  A: 

Don't build the where clause before it is needed to execute the search. You may end up with a user interface that feeds parameters in iterations, and you don't know when you have everything. Also, you may never execute the search, so why worry about the where clause.

cdonner
A: 

On an SQLServer database it is more efficient to include all parameters in the where clause rather than building it on the fly. Then have an database index which includes all the columns you will search on. This ensures the index is always used when executing the statement.

bstoney
+3  A: 

In your method, simply use the parameters in your dynamic SQL that does the search. That way the where clause is built just prior to the SQL getting run. You will simply pass the search parameters into your method as arguments.

Something like this...

<cffunction name="getByAttributesQuery" access="public" output="false" returntype="query">
 <cfargument name="id" type="numeric" required="false" />
 <cfargument name="userName" type="string" required="false" />
 <cfargument name="firstName" type="string" required="false" />
 <cfargument name="lastName" type="string" required="false" />
 <cfargument name="createdAt" type="date" required="false" />
 <cfargument name="updatedAt" type="date" required="false" />
 <cfargument name="orderby" type="string" required="false" />

 <cfset var qList = "" />  
 <cfquery name="qList" datasource="#variables.dsn#">
  SELECT 
   id,
   userName,
   firstName,
   lastName,
   createdAt,
   updatedAt
  FROM users
  WHERE  0=0
 <cfif structKeyExists(arguments,"id") and len(arguments.id)>
  AND id = <cfqueryparam value="#arguments.id#" CFSQLType="cf_sql_integer" />
 </cfif>
 <cfif structKeyExists(arguments,"userName") and len(arguments.userName)>
  AND userName = <cfqueryparam value="#arguments.userName#" CFSQLType="cf_sql_varchar" />
 </cfif>
 <cfif structKeyExists(arguments,"firstName") and len(arguments.firstName)>
  AND firstName = <cfqueryparam value="#arguments.firstName#" CFSQLType="cf_sql_varchar" />
 </cfif>
 <cfif structKeyExists(arguments,"lastName") and len(arguments.lastName)>
  AND lastName = <cfqueryparam value="#arguments.lastName#" CFSQLType="cf_sql_varchar" />
 </cfif>
 <cfif structKeyExists(arguments,"createdAt") and len(arguments.createdAt)>
  AND createdAt = <cfqueryparam value="#arguments.createdAt#" CFSQLType="cf_sql_timestamp" />
 </cfif>
 <cfif structKeyExists(arguments,"updatedAt") and len(arguments.updatedAt)>
  AND updatedAt = <cfqueryparam value="#arguments.updatedAt#" CFSQLType="cf_sql_timestamp" />
 </cfif>
 <cfif structKeyExists(arguments, "orderby") and len(arguments.orderBy)>
  ORDER BY #arguments.orderby#
 </cfif>
 </cfquery>

 <cfreturn qList />
</cffunction>
Russ Johnson
Drop the len(...) checks - they should not be there.
Peter Boughton
I need to set the parameters for the search before doing the actual search, so I cant simply pass all the parameters in to the doSearch method. I also think that by providing setter methods outside of the doSearch method gives me a more flexible orderSearch object
Yisroel
>> I need to set the parameters for the search before doing the actual search, so I cant simply pass all the parameters in to the doSearch method <<Huh?Not even with something like doSearch( ArgumentCollection = Variables.SearchData ) ?
Peter Boughton
+1  A: 

I don't think it makes much difference, but I think it seems better practice to build the WHERE clause when you doSearch. I don't think it should be the responsibility of a setter for a parameter to add to a WHERE clause string somewhere.

MikeW
A: 

Dosn't quite sound like your abstraction is quite right.

Search may be better as a method of a "orders" object. pass in the parameters to the search function and build the query manually as russ suggested. Anything which is specific to orders rather than the search can then be set on the orders init method.

It is possible that you may want to build an orders search object but this should be done via an orders object, to keep your front end code simple.

Jeremy French
What functionality is the Orders object providing?
Yisroel
A: 

Option 1 is your best bet. Option 2 sounds dangerous. What if a parameter is updated? How do you replace it in your WHERE clause?

I would do something like this:

<cffunction name="doSearch" access="public" output="false" returntype="query">        
    <cfset var qList = "" />
    <cfquery name="qList" datasource="#variables.dsn#">
       SELECT
           ...
       FROM
           ...
       WHERE 0=0 
         <cfif len(getID())>
           AND id = <cfqueryparam value="#getID()#" CFSQLType="cf_sql_integer" />
         </cfif>       
         <cfif len(getUserName())>
           AND userName = <cfqueryparam value="#getUserName()#" CFSQLType="cf_sql_varchar" />
         </cfif>
    </cfquery>
    <cfreturn qList />
</cffunction>
Al Everett
I dont have to worry about updating a parameter, as a new orderSearch is instatiated for each search. In essence, I'm doing exactly as you do above. I just wanted a clear understanding of whats wrong with the other option.btw, you call getUserName(), but then reference arguments.userName
Yisroel
+14  A: 

You should separate the code for your order search from the code that builds the SQL. The SQL should be built in a derivative (or Strategy derivative) of the OrderSearch class. Once you have made this separation, it doesn't really matter when you build the SQL.

To make this a bit more plain. Given a class named OrderSearch which has a bunch of setter methods for the search criteria, you'd like to have a subclass named OrderSearchSQLBuilder. Notice that the subclass depends on the base class, and that the base class is independent of the subclass. This is very important. This independence allows you to disregard whether the SQL is built in the setter methods, or in the search method. See The Dependency Inversion Principle (DIP).

Once you have this kind of seperation, you can replace the derivative with other strategies. For example, if you wanted to test your application without connecting it to the SQL database, you could create a dummy in-ram database and create a derivative of OrderSearch that dealt with that dummy database. The rest of the application would be blissfully unaware, and your tests would then be independent of the horrors of database connections, pre-existing data, etc.

Uncle Bob