CODEWITHSUNDEEP
phpif-statementrefactoring
Joel
Joel

Reputation: 2721

How do I clean up this if/else statement? (refactoring)

I know there must be a nicer way to do this, but whenever I search "&&" i don't get good enough results...

<?php
if (empty($row[ContactName]) && empty($row[ContactEmail]) && empty($row[ContactPhone]) && empty($row[Website])){
echo "Not Provided";
}
else{
...do stuff...
}
?>

Thanks!

Upvotes: 5

Views: 1527

Answers (6)

bugwheels94
bugwheels94

Reputation: 31920

<?php
$i=1;
$ar=array('ContactName','ContactEmail','ContactPhone','Website')

foreach($ar as $a)
  if (empty($row[$a]))
    {
     $i=0;
     break;                      //to make code fast
    }
  if($i)         //do stuff
  else echo 'not provided';
?>

or if you really want to make your code extra small then change your column name in database

From               To
ContactName        Col1
ContactEmail       Col2
ContactPhone       Col3
Website            Col4

and then do

 <?php
  $i=1;
  for($a=1;$a<5;$a++)
    if (empty($row['Col'.$a]))
    {
      $i=0;
      break;
    }
  if($i)//do stuff
  else echo 'Not Provided';
    ?>

However it is not good to rename column as it will make your db somewhat less understandable.

Upvotes: 5

T.G
T.G

Reputation: 1921

Do not play with making arrays and stuff just to make code look nicer. As Tommy pointed

<?php
if (empty($row[ContactName]) 
    && empty($row[ContactEmail]) 
    && empty($row[ContactPhone]) 
    && empty($row[Website]))
{
    echo "Not Provided";
} 
else{
 ...do stuff...
}
?>

this code is nice, just need proper formatting like he pointed. Making arrays and then launching loop will decrease performance

<?php
    $i=1;
    for($a=1;$a<5;$a++)
    if (empty($row['Col'.$a]))
    {
    $i=0;
    break;
    }
    if($i)//do stuff
    ?>

this might look small, nice and "pro", but has more operations to give same results as a simple if instruction. Not saying that it cant be done faster, i don't know php well, just remember about code executing speed.

Upvotes: 0

WebChemist
WebChemist

Reputation: 4411

you could make it as short as this if compactness was more important than readability :)

$required = array('ContactName', 'ContactEmail', 'ContactPhone', 'Website');

$ra = array_filter(array_intersect_key($row,array_flip($required)));
if(empty($ra)){
    echo "Not Provided";
}
else{
    //....
}

you can't put array_filter inside empty() or you get "Fatal error: Can't use function return value in write context"

1) array flip is turning your $required values into keys
2) array_intersect_key throws away any $row keys not found in $required
3) array_filter throws away any empty values

leaving you with a zero length array, which is empty. No need to worry about array lengths or loops

Upvotes: 1

Ali
Ali

Reputation: 22317

As php functions have a lot of inconsistency, It's always a good idea using a library to make it more consistent.

put this function in that library:

function empty()
{
    foreach(func_get_args() as $arg) if (empty($arg)) return false;
    return true;
}

usage:

if (empty($row[ContactName], $row[ContactEmail], $row[ContactPhone], $row[Website])) {
      echo "Not Provided";
}

Upvotes: 1

coolguy
coolguy

Reputation: 7954

<?php
$flag = 1;
if (empty($row[ContactName])){
$flag = 0;
}
if (empty($row[ContactEmail])){
$flag = 0;
}
if (empty($row[ContactPhone])){
$flag = 0;
}
if (empty($row[Website])){
$flag = 0;
}

if($flag == 0){
echo "Not Provided";

}else{
//do stuff
}    


?>

OR

$flag = 1;
$i=0;
$mustarray = array('ContactName','ContactName','ContactName','ContactName'); //or any number of fields that you want as mandatory
foreach($yourresult as $row){
   if(empty($row[$mustarray [$i]])){
    $flag = 0;
} 
$i++;
}
    if($flag == 0){
    echo "Not Provided";

    }else{
    //do stuff
    }

Upvotes: 0

Tommy
Tommy

Reputation: 13632

What is wrong with the original code? 

<?php
if (empty($row[ContactName]) 
    && empty($row[ContactEmail]) 
    && empty($row[ContactPhone]) 
    && empty($row[Website]))
{
    echo "Not Provided";
} 
else{
 ...do stuff...
}
?>

Looks like fine code to me...

Upvotes: 6

Related Questions