Which one is better? case when or if else?

Question

My case is like this :

case when is like this :

public function get_opportunity($customer_id, $group_id, $action)
{
    $sql = "SELECT COUNT(transaction_id) AS total_transaction 
            FROM `transaction` 
            WHERE 
                CASE 
                    WHEN '$action' = 'view_all' THEN transaction_id IS NOT NULL
                    WHEN '$action' = 'view_group' THEN group_id IN($group_id) 
                    ELSE transaction_created_by = ? 
                END 
            ";

    $result = $this->db->query($sql, array($customer_id))->result_array();

    return ($result[0]['total_transaction']) ? $result[0]['total_transaction'] : 0;
}

if else is like this :

public function get_opportunity($customer_id, $group_id, $action)
{
    if($action == "view_all")
        $condition = " transaction_id IS NOT NULL";
    else if($action == 'view_group')
        $condition = " group_id IN($group_id)";
    else
        $condition = " customer_id = ?";

    $sql = "SELECT COUNT(transaction_id) AS total_transaction 
            FROM `transaction` 
            WHERE $condition";

    $result = $this->db->query($sql, array($customer_id))->result_array();

    return ($result[0]['total_transaction']) ? $result[0]['total_transaction'] : 0;
}

I have tried it. The all type of the above work. But here I ask, which one is better?

Why are you putting a condition based on a PHP variable inside your MySQL code? Figure out which query you want to execute, then run the correct one. Don't dump this on your database. — tadman, Jul 26 '16 at 06:52
Also, please, **DO NOT** inline arbitrary values. Use [prepared statements with placeholder values](http://stackoverflow.com/questions/10968527/escaping-sql-queries-in-codeigniter) whenever possible. — tadman, Jul 26 '16 at 06:53
This is a function of his code i guess.. just compare which query would be better to use. a combination lols — Vijunav Vastivch, Jul 26 '16 at 06:54
Generally in order of desirability from most to least: Lookup through an associative `array` (e.g. `array('view_all' => 'transaction IS NOT NULL', ...)`), [`switch` statement](http://php.net/manual/en/control-structures.switch.php), long chain of `if` statements. — tadman, Jul 26 '16 at 06:57
@tadman, I have tried both of the above and it worked. I use mysql database — samuel toh, Jul 26 '16 at 07:07
@tadman, to prepare statements issue, I actually used this: `group_id IN(?)`, but if in the last query, the results `group_id IN('1,2,3')`. No `group_id IN(1,2,3)`. I finally decided to use `group_id IN ($group_id)` — samuel toh, Jul 26 '16 at 07:24

sagi · Accepted Answer · 2016-07-26T06:58:56.983

2

That can't be right, the first code should not work! In SQL CASE is an expression, not a statement. You can't put a condition on the THEN part, only a single value. MySQL evaluates Boolean expressions as 1\0 , so your case is basically like this:

WHERE CASE... THEN (Cond) -- equals to 1/0 
              THEN (Cond) -- equals to 1/0
              ELSE (Cond) -- -- equals to 1/0

Most RDBMS will throw an error.

You can format it like this:

SELECT COUNT(transaction_id) AS total_transaction 
            FROM `transaction` 
            WHERE ('$action' = 'view_all' AND transaction_id IS NOT NULL) OR 
                  ('$action' = 'view_group' THEN group_id IN($group_id)) OR
                  transaction_created_by = ?

As long as you don't have performance issues, don't think about this things, it will just waste you time. Pick the one that will be easier for you to maintain.

edited Jul 26 '16 at 06:58

answered Jul 26 '16 at 06:52

sagi

40,026
6
59
84

Thank you very much. I have tried both of the above and it worked. I use mysql database – samuel toh Jul 26 '16 at 07:06
How do you think the second way (if else)? if it is good to use? – samuel toh Jul 26 '16 at 07:14
Yes, it's ok, like @dburmann said, it can lead to SQL-Injection, but in general, choose the one easier to maintain. They are both good, but sometimes doing calculation and logic on the site side is preferred. – sagi Jul 26 '16 at 07:53

dbrumann · Answer 2 · 2016-07-26T07:20:17.693

Apart from the issue raised by @sagi actually the second approach is better for a somewhat unrelated reason. In the first example you use the variable $action directly inside the query-string which might open an attack vector for an SQL injection as it comes from outside the method scope and might not be safe. Whereas in the second example the variable $condition is controlled in the method and safe to use.

Apart from that I would go with readability as long as performance is not an issue. For that reason I might even split this up into 3 separate, clearly named functions instead which have a very similar SQL. That way it is always clear what's happening and each case can "grow" independently instead of 1 method where more and more cases and ifs are added.

edit: Also you can name the function clearly for what it does, e.g.

getAllTransactionCounts()
getTransactionCountForCustomers($transactionIds)
getTransactionCountForCustomer($customerId)

@samueltoh I don't see major problems with the second approach, but some refactoring is still needed. For example what happens when both customer_id and action are null? What if group_id is provided as array, not comma separated list? That's why I suggested splitting it up in 3 methods (maybe private methods called by `get_opportunity`) with only the necessary arguments to make these cases clearer and easier to manage. — dbrumann, Jul 26 '16 at 07:50

Which one is better? case when or if else?

2 Answers2