views:

106

answers:

8

I have to build a query based on certain conditions. Is there a better way of doing it than the way I have done below? It works fine but I can see it getting out of hand fairly quickly if there were more conditions since I check if any previous conditions had been met every time I check a new one.

    $sql = "SELECT DISTINCT fkRespondentID FROM tblRespondentDayTime";

    if (!empty($day) || !empty($time) || !empty($sportID)) {

        $sql .= " WHERE";

        if (!empty($day)) {
            $sql .= " fldDay='$day'";
        }

        if (!empty($time)) {
            if (!empty($day)) {
                $sql .= " AND";
            }
            $sql .= " fldTime='$time'";
        }

        if (!empty($sportID)) {
            if (!empty($day) || !empty($time)) {
                $sql .= " AND";
            }
            $sql .= " fkRespondentID IN (SELECT fkRespondentID FROM tblRespondentSport WHERE fkSportID='$sportID')";
        }

    }
A: 

you could try putting your variables in an array and having a boolean that tells if you need to add the "AND" in before your next phrase. This would shorten your control statements to a foreach and a nested if.

Scott M.
+5  A: 

I would use the old "WHERE 1=1" trick; add this as the first condition, and then you can assume the "AND" condition on each statement that follows.

DanP
@Palpie has demonstrated practical usage of this method in his answer.
DanP
A: 

Here is my solution:

$sql = "SELECT * FROM table";
$conditions = array(
  'fldDay' => $day,
  'fldTime' => $time,
);

if (count(array_filter($conditions))) {
  $sql .= ' WHERE ';
  $sql .= implode(' AND ', array_map(function($field, $value) {
    return $field . '=\'' . pg_escape_string($value) . '\'';
  }, array_keys($conditions), $conditions));
}

Please note, that because of the closures, this won't work below PHP 5.3. If you are using an older PHP, make the closure as a separate function, or substitute it with a foreach.

Yorirou
This code is hard to read.
FractalizeR
+1  A: 

Build a list/array of conditions, where each conditional is optional (i.e. if condition is valid, push it on the list).

If this list is > 0, add "where" and then add the list join'ed by "and".

Mark Canlas
A: 

Unfortunately, building dynamic SQL is a tedious experience and even if you can change a few things in your logic (which actually looks relatively clean), it's still going to be ugly.

Fortunately, Object-relational mapping exists. I'm not too familiar with PHP, but Perl has several CPAN modules such as SQL::Abstract, which will allow you to build fairly complex SQL statements using basic data structures.

bowenl2
We don't know the size of the system birderic is creating. May be involving ORMs and SQL constructors is not optimal here.
FractalizeR
+1  A: 

Rather than doing checks like if (!empty($day) || !empty($time)) you can create a $whereClause variable and check it like this:

$sql = "SELECT DISTINCT fkRespondentID 
        FROM tblRespondentDayTime";

$whereClause = '';

// fldDay
if (!empty($day)) {
    $whereClause .= " fldDay='$day'";
}

// fldTime
if (!empty($time)) {
    if (!empty($whereClause)) {
        $whereClause .= ' AND ';
    }
    $whereClause .= " fldTime='$time'";
}

// fkRespondentID
if (!empty($sportID)) {
    if (!empty($whereClause)) {
        $whereClause .= ' AND ';
    }
    $whereClause .= " fkRespondentID IN (SELECT fkRespondentID 
                                         FROM tblRespondentSport 
                                         WHERE fkSportID='$sportID')";
}

if (!empty($whereClause)) {
    $whereClause = ' WHERE '.$whereClause;
}

$sql .= $whereClause;

This will also work if you need to, say, change some to an OR (1=1 trick won't work in that case and could even prove quite hazardous).

webbiedave
+2  A: 
$sql = "SELECT DISTINCT fkRespondentID FROM tblRespondentDayTime WHERE 1=1";

if (!empty($day))
    $sql .= "AND fldDay='$day'";

if (!empty($time)) {
    $sql .= "AND fldTime='$time'";

if (!empty($sportID))
    $sql .= "AND fkRespondentID IN (SELECT fkRespondentID FROM tblRespondentSport WHERE fkSportID='$sportID')";
Palpie
@Palpie: Thanks for the example implementation + 1
DanP
A: 

If you use stored procedures, you can do something like this:

CREATE PROCEDURE `FindRespondents` (
    IN `_day` varchar(255),
    ...
)
BEGIN
    SELECT DISTINCT fkRespondentID 
    FROM tblRespondentDayTime
    WHERE (_day Is Null OR fldDay = _day)
        AND ...
END;
|

Passing in null for _day means any fldDay is OK. Any other value for _day, and it must be matched. I assumed fldDay is text, but of course you can type everything properly here.

I know some people aren't fans of stored procedures, but it can be handy encapsulate query logic this way.

grossvogel
I guess it'd work with prepared statements, too, if you write the where clause out this way, and bind each of those parameters to `?`s.
grossvogel