Optimize this code snippet

PHP
1

I have the following snippet that builds a SQL query.
Essentially there are 4 checkboxes that allow a user to filter results returned from a MySQL query.


	$switch = false;
	if(!isset($payment_status['paid']) || !isset($payment_status['pending']) || !isset($payment_status['failed']) || !isset($payment_status['refunded'])) { //One of them is unset so we can act
		if(isset($payment_status['paid']) && $switch == false) {
			$switch = true;
			$sql .= " AND (orders.payment_status = 'paid'";
			if(isset($payment_status['pending'])) {
				$sql .= " OR orders.payment_status = 'pending'";
			}
			if(isset($payment_status['failed'])) {
				$sql .= " OR orders.payment_status = 'failed'";
			}
			if(isset($payment_status['refunded'])) {
				$sql .= " OR orders.payment_status = 'refunded'";
			}
			$sql .= ")";
		}
		if(isset($payment_status['pending']) && $switch == false) {
			$sql .= " AND (orders.payment_status = 'pending'";
			if(isset($payment_status['failed'])) {
				$sql .= " OR orders.payment_status = 'failed'";
			}
			if(isset($payment_status['refunded'])) {
				$sql .= " OR orders.payment_status = 'refunded'";
			}
			$sql .= ")";
		}
		if(isset($payment_status['failed']) && $switch == false) {
			$sql .= " AND (orders.payment_status = 'failed'";
			if(isset($payment_status['refunded'])) {
				$sql .= " OR orders.payment_status = 'refunded'";
			}
			$sql .= ")";
		}
		if(isset($payment_status['refunded']) && $switch == false) {
			$sql .= " AND orders.payment_status = 'refunded'";
		}
	}

Essentially I’ve had to outline every combination of clicked checkboxes because if there’s more then one checkbox clicked the statement syntax changes.

Cheers!

2

That’s an interesting way to do it! So you’re forcing the OR by putting 0=1 which will always be untrue. Am I understanding that right?

3

I think you could replace all that with



if(!isset($payment_status['paid']) || !isset($payment_status['pending']) || !isset($payment_status['failed']) || !isset($payment_status['refunded'])) { //One of them is unset so we can act
    
    $sql .= " AND (0 = 1";
    if (isset($payment_status['paid'])) {
        $sql .= " OR orders.payment_status = 'paid'";
    if (isset($payment_status['pending'])) {
        $sql .= " OR orders.payment_status = 'pending'";
    }
    if (isset($payment_status['failed'])) {
        $sql .= " OR orders.payment_status = 'failed'";
    }
    if (isset($payment_status['refunded'])) {
        $sql .= " OR orders.payment_status = 'refunded'";
    }
    $sql .= ")";

}
4

Yes that’s right. The truth table is the same; each row must match at least one of the checked statuses to be selected.

5

Unreal, thanks for the help!

6

or you could just cat them all together and stick them in an IN(), and do it in 2 lines…(EDIT: 1 line.)


$sql .= " AND orders.payment_status IN('".implode("','",array_keys($payment_status))."')";
7

Now that’s what I’m talking about! Thanks to both of you for your input.