Using a certain query field based on state

Question

I need to be able to do a select to a table using a where clause that can use an email or phone field, depending on if they are set/not empty or not. Given this context, how can I simplify this PHP code?

 if(isset($email) && !empty($email) && isset($phone) && !empty($phone)){
 $partQuery = 'account_id = '.$account->id.' AND (phone = "'.(int)$phone.'" OR email = "'.$email.'")';
 }
 if(isset($phone) && !empty($phone)){
 $partQuery = 'account_id = '.$account->id.' AND phone = "'.(int)$phone.'"';
 }
 if(isset($email) && !empty($email)){
 $partQuery = 'account_id = '.$account->id.' AND email = "'. $email.'"';
 }

• Mathieu Guindon: Welcome to Code Review! I'm not the downvoter, but I think it would make a much better post if you included more contextual code, included a bit of a description of what your code is achieving, and changed the title to something a bit less general - everybody on this site wants to make their code better ;)
• hopper: @Mat'sMug done!

Answer 1

First of all, use prepared statements.

The account id part is always the same, so extract it to the beginning.

Also, you can extract the isset and not empty part to a function to simplify the code.

$partQuery = 'account_id = '.$account->id;
if(isNotEmpty($phone) && isNotEmpty($email)) {
 $partQuery .= ' AND (phone = "'.(int)$phone.'" OR email = "'.$email.'")';
}
if(isNotEmpty($phone)) {
 $partQuery .= ' AND phone = "'.(int)$phone.'"';
}
if(isNotEmpty($email)) {
 $partQuery .= ' AND email = "'. $email.'"';
}
function isNotEmpty($var) {
 return isset($var) && !empty($var);
}

Although I would assume that that is not actually what you want, as it adds two additional ANDs in case neither phone nor email is empty.

It also seems like an odd query in general. You are fine with a wrong phone number as long as the email is correct and vise versa?

Also note that you don't actually need isset, you can just use !empty, it will not generate a warning.

Answer 2

Just a suggestion: I often like to use arrays for this kind of work. Like this:

// it must be this account
$andArray[] = 'account_id = '.$account->id;
// match either the email or phone number
if (isNotEmpty($email)) $orArray[] = 'email = "'.$email.'"';
if (isNotEmpty($phone)) $orArray[] = 'phone = "'.(int)$phone.'"';
if (isset($orArray)) $andArray[] = '('.implode(' OR ',$orArray).')';
// create this part of the query
$partQuery = implode(' AND ',$andArray);

it tends to keep the code a lot shorter, especially when it becomes more complex. Yes, I used the isNotEmpty() from tim, it's what you need here.