Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

So a situation I run into reasonably often is one where my models start to either:

  • Grow into monsters with tons and tons of methods

OR

  • Allow you to pass pieces of SQL to them, so that they are flexible enough to not require a million different methods

For example, say we have a "widget" model. We start with some basic methods:

  • get($id)
  • insert($record)
  • update($id, $record)
  • delete($id)
  • getList() // get a list of Widgets

That's all fine and dandy, but then we need some reporting:

  • listCreatedBetween($start_date, $end_date)
  • listPurchasedBetween($start_date, $end_date)
  • listOfPending()

And then the reporting starts to get complex:

  • listPendingCreatedBetween($start_date, $end_date)
  • listForCustomer($customer_id)
  • listPendingCreatedBetweenForCustomer($customer_id, $start_date, $end_date)

You can see where this is growing... eventually we have so many specific query requirements that I either need to implement tons and tons of methods, or some sort of "query" object that I can pass to a single ->query(query $query) method...

... or just bite the bullet, and start doing something like this:

  • list = MyModel->query(" start_date > X AND end_date < Y AND pending = 1 AND customer_id = Z ")

There's a certain appeal to just having one method like that instead of 50 million other more specific methods... but it feels "wrong" sometimes to stuff a pile of what's basically SQL into the controller.

Is there a "right" way to handle situations like this? Does it seem acceptable to be stuffing queries like that into a generic ->query() method?

Are there better strategies?

share|improve this question
    
I'm going through this same problem right now in a non-MVC project. The question keeps coming up of should the data access layer abstract out every stored procedure, and leave the business logic layer database agnostic, or should the data access layer be generic, at the cost of the business layer knowing something about the underlying database? Perhaps an intermediate solution is to have something like ExecuteSP(string spName, params object[] parameters), then include all of the SP names in a config file for the business layer to read. I don't really have a very good answer to this, though. –  Greg Jackson Apr 6 '12 at 15:50
add comment

6 Answers

There's no correct way to do this. Many people use ORMs to abstract away all the complexity. Some of the more advanced ORMs translate code expressions into complicated SQL statements. ORMs have their downsides as well, however for many applications the benefits outweigh the costs.

If you aren't working with a massive dataset, the simplest thing to do is to select the entire table into memory and filter in code.

//pseudocode
List<Person> people = Sql.GetList<Person>("select * from people");
List<Person> over21 = people.Where(x => x.Age >= 21);

For internal reporting applications this approach is probably fine. If the dataset is really big, you'll start needing lots of custom methods as well as appropriate indexes on your table.

share|improve this answer
1  
+ 1 for "There's no correct way to do this" –  Ozz Apr 6 '12 at 9:53
1  
Unfortunately, filtering outside of the data set isn't really an option with even the smallest data sets we work with- it's just too slow. :-( Good to hear that others run into my same problem though. :-) –  Keith Palmer Apr 6 '12 at 11:07
    
@KeithPalmer out of curiosity, how big are your tables? –  dan Apr 7 '12 at 4:22
    
Hundreds of thousands of rows, if not more. Too many to filter with acceptable performance outside the database, ESPECIALLY with a distributed architecture where the databases are not on the same machine as the application. –  Keith Palmer Apr 11 '12 at 17:22
    
awesome! good luck –  dan Apr 12 '12 at 0:07
show 2 more comments

Some ORMs allow you to construct complex queries starting from basic methods. For instance

old_purchases = (Purchase.objects
    .filter(date__lt=date.today(),type=Purchase.PRESENT).
    .excude(status=Purchase.REJECTED)
    .order_by('customer'))

is a perfectly valid query in the Django ORM.

The idea is that you have some query builder (in this case Purchase.objects) whose internal status represents information about a query. Methods like get, filter, exclude, order_by are valid and return a new query builder with an updated status. These objects implement an iterable interface, so that when you iterate over them, the query is performed and you get the results of the query constructed so far. Although this example is taken from Django, you will see the same structure in many other ORMs.

share|improve this answer
    
I don't see what advantage this has over something like old_purchases = Purchases.query("date > date.today() AND type = Purchase.PRESENT AND status != Purchase.REJECTED"); You're not reducing complexity or abstracting anything by just making SQL ANDs and ORs into method ANDs and ORs- you're just changing the representation of the ANDs and ORs, right? –  Keith Palmer Apr 6 '12 at 11:10
4  
Actually not. You are abstracting away the SQL, which buys you a lot of advantages. First, you avoid injection. Then, you can change the underlying database without worrying about slightly different versions of the SQL dialect, since the ORM handles this for you. In many cases, you can also put a NoSQL backend without noticing. Third, these query builders are objects that you can pass around like anything else. This means that your model can construct half the query (for instance you could have some methods for the most common cases) and then it can be refined in the controller to handle the.. –  Andrea Apr 6 '12 at 12:26
2  
...most specific cases. A typical example is defining a default ordering for models in Django. All query results will follow that ordering unless you specify otherwise. Fourth, if you ever need to denormalize your data for performance reason, you only have to tweak the ORM rather than rewriting all your queries. –  Andrea Apr 6 '12 at 12:31
    
+1 For dynamic query languages like the one mentioned, and LINQ. –  Evan Plaice Apr 7 '12 at 1:27
add comment

I think the general consensus is to keep as much as data access as possible in your models in MVC. One of the other design priciple is to move some of your more generic queries (Those which are not directly related to your model) to a higher, more abstract level where you can allow it to be used by other models as well. (In RoR, we have something called framework) There is also another thing that you have to consider and that is the maintanability of your code. As your project grows, if you have data access in controllers, it will become increasingly difficult to track it down(We are currently facing this issue in a huge project) Models, although cluttered with methods provide a single point of contact for any controller who might end up quering from the tables. (This may also lead to a reuse of code which is in turn beneficial)

share|improve this answer
1  
Example of what you're talking about...? –  Keith Palmer Apr 6 '12 at 11:27
add comment

Martin Fowler's Patterns of Enterprise Application Architecture describes a number of ORM related paterns, including use of the Query Object, which is what I'd suggest.

Query objects let you follow the Single Responsibility principle, by separating the logic for each query into individually managed and maintained strategy objects. Either your controller can manage their use directly, or delegate that to a secondary controller or helper object.

Will you have a lot of them? Certainly. Can some be grouped into generic queries? Yes again.

Can you use dependency injection to create the objects from metadata? That's what most ORM tools do.

share|improve this answer
add comment

Your service layer interface may have many methods, but the call to the database can have just one.

A database has 4 major operations

  • Insert
  • Update
  • Delete
  • Query

Another optional method may be to execute some database operation that doesn't fall under the basic DB operations. Let's call that Execute.

Insert and Updates can be combined into one operation, called Save.

Alot of your methods are query's. So you can create a generic interface to satisfy most of the immediate needs. Here's a sample generic interface:

 public interface IDALService
    {
        DataTransferObject<T> Save<T>(DataTransferObject<T> Dto) where T : IPOCO;
        DataTransferObject<T> Search<T>(DataTransferObject<T> Dto) where T: IPOCO;
        DataTransferObject<T> Delete<T>(DataTransferObject<T> Dto) where T : IPOCO;
        DataTransferObject<T> Execute<T>(DataTransferObject<T> Dto) where T : IPOCO;
    }

The data transfer object is generic and would have all your filters, parameters, sorting, etc. contained within it. The data layer would be responsible for parsing and extracting this out and setting up the operation to the database via stored procedures, parameterized sql, linq etc. So, SQL is not passed between layers. This is typically what an ORM does, but you can roll your own and have your own mapping.

So, in your case you have Widgets. Widgets would implement the IPOCO interface.

So, in your service layer model would have getList().

Would need a mapping layer to handle tranforming getList into

Search<Widget>(DataTransferObject<Widget> Dto)

and vice versa. As other have mentioned, sometime this is done via an ORM, but ultimately you end up with a lot of boilerplate type of code, especially if you have 100s of tables. The ORM magically creates parametized SQL and runs that against the database. If rolling your own, additionally in the data layer itself, mappers would be needed to set up the SP, linq etc. (Basically the sql going to the database).

As mentioned earlier, the DTO is a object made up by composition. Perhaps one of the objects contained within it is an object called QueryParameters. These would be all the parameters for the query which would be set up and used by the query. Another object would be a List of returned objects from querys, updates, ext. This is the payload. If this case the payload would be a List of widgets List.

So, the basic strategy is:

  • Service Layer Calls
  • Transform Service Layer Call To Database using some sort of Repository/Mapping
  • Database Call

In your case I think the model could have many methods, but optimally you want the database call to be generic. You still end up with lots of boilerplate mapping code (especially with SPs) or magical ORM code that is dynamically creating the parametized SQL for you.

share|improve this answer
add comment

There's a third approach.

Your specific example exhibits exponential growth in the number of methods needed as the number of required features grows: we want the ability to offer advanced queries, combining every query feature...if we do that by adding methods, we have one method for a basic query, two if we add one optional feature, four if we add two, eight if we add three, 2^n if we add n features.

That's obviously unmaintainable beyond three or four features, and there's a bad smell of a lot of closely related code that's almost copy-pasted between methods.

You could avoid that by adding a data object to hold the parameters, and have a single method that builds the query based on the set of parameters provided (or not provided). In that case, adding a new feature such as a date range is as simple as adding setters and getters for the date range to your data object, and then adding a bit of code where the parameterized query is built:

if (dataObject.getStartDate() != null) {
    query += " AND (date BETWEEN ? AND ?) "
}

...and where the parameters are added to the query:

if (dataObject.getStartDate() != null) {
    preparedStatement.setTime(dataObject.getStartDate());
    preparedStatement.setTime(dataObject.getEndDate());
}

This approach allows for linear code growth as features are added, without having to allow arbitrary, unparameterized queries.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.