OOP DAO design question

Question

Quick background: I'm programming in PHP, I have a domain model with a separate data access layer (DAO classes) that are responsible for fetching data from the DB and creating domain classes.

Let's say I have a DAO class responsible for creating group and groupList objects. You can imagine the groups as a component of a social network; although it doesn't really matter what they are for this question.

I need to be able to ask the DAO to make me various groupList objects based on various different criteria:

• the most recently added groups
• the most popular groups
• groups identified as "featured" by an administrator
• groups tagged with a certain tag
• groups matching a certain keyword
• groups within a certain category
• groups created by a certain person
• groups created on a certain day

Some of these I don't actually need right now, but I can kind of imagine I'll need them before the project's done. Now I started off with a nice simple DAO method: createList. This worked great. You can think of the pseudo code as:

find out how many groups
create SQL query to fetch group details
loop through results
{
   create group object
   add to group list object
}

As my application progressed, I then created a new method createFeaturedList. This worked great. But it was actually very very similar to createList, with a slightly different query. A lot of the rest of the ~ 150 lines of code were identical.

So... what should I do about all the slightly different cases I need? Most of the time I really just want to filter and sort the list based on certain criteria. The question is this -- should I:

a) create lots of focused creational method like:

• createList()
• createCategoryList( categoryObject )
• createUsersList ( userObject )
• createTagList ( tag )
• createPopularList ()

or

b) create one BIG method that can do everything: - createList ( searchString, orderBy, filterByCategoryObject=null, filterByUserObject=null )

I quite like the idea of (a) because my DAO interface is simpler and less likely to need to change (eg: adding another parameter when I suddenly need to pass in a date to compare against) The difficulty comes when you have stuff like search keywords which you want to be able to combine with other parameters; eg: searched category list, searched popular list, searched tag list etc... (a) is kind of what I've started off with.

I've had a flirt with refactoring to (b) but I can see my method getting very large and very complex pretty quick, lots of "if"s and "select"s to deal with the different cases when building the SQL, and lots of parameters that are fed into the method. But at least it's all in one place. And you can combine things; eg: a user's groups, tagged with blah, matching keywords blah.

Answer 1

Programming fundamentals: it's good practice to break down your codes into sections or functions.

I will go for the option (a); You will need to maintain and debug the code. And when you do have a bug, you will be very happy that you have split up your code into various methods.

In addition, writing the method name helps you to understand what you are doing.

compare this:

Option (a)

$obj->AddNewList( /* params */ );
$obj->UpdateList( /* params */ );

and this:

Option (b)

$obj->parse( /* first set of params */ );
$obj->parse( /* second set of params */ );

It saves time as humans read from left to right. That's why function and method names are always found on the left.

Answer 2

If either performance isn't a major issue, or the groups change sufficiently slowly to enable query caching to be useful you could write filtering functions and pass those in. If you're looping through groups and have an optional array of filtering methods your loop would become:

for(group in group) {
    cont = true
    for(f in functions) {
        if ! f(group) {cont = false; continue;}
    }
    if(cont) Continue
    add group to list
}

This would allow you to alter the filtering params without altering the loop, just writing or altering a function.

Answer 3

You could make a private method that all the public methods call into. IE

private function _createList ( searchString, orderBy, ... )
{
    ...
}

public function createList()
{
    return $this->_createList('...', 'id');
}

public function createCategoryList()
{
    return $this->_createList('...', 'category_id');
}

That way if your _createList function needs to change later you only have to refactor the public methods in this DAO rather than all classes that use this DAO.

Answer 4

I don't think it's strictly an either or situation. Option a is a nice usable interface for your DAO to expose so I think you should keep that. To me Option b really seems like the implementation specific logic. So if the BIG method suits your purposes, I would say to use it to perform the actual processing logic while exposing the interface as in option a.

That said, if the BIG method becomes too convoluted and complicated and the code reuse is actually increasing your code complexity and decreasing application maintainability then you might want to refactor to keep separate SQL statements for each interface method but have the helper method execute the common logic of parsing the results.

Answer 5

The big method in option B is almost guaranteed to decrease code reuse, and increase complexity and maintenance time.

Personally, (and according to Code Complete) methods should do one thing and do it well and not try to cram everything in. Avoid having to refactor in the future and do it smart the first time.