-4
\$\begingroup\$

I'm putting together a little application and it requires about 6 if statements to verify actions, here's the code I'm using:

if($player[0]['value'] == $server->clean_attr($row['attr1'])) {
    if($player[1]['value'] == $server->clean_attr($row['attr2'])) {
        if($player[2]['value'] == $server->clean_attr($row['attr3'])) {
            if($player[3]['value'] == $server->clean_attr($row['attr4'])) {
                if($player[4]['value'] == $server->clean_attr($row['attr5'])) {
                    if($player[5]['value'] == $server->clean_attr($row['attr6'])) {
                        //perform action here
                    }
                }
            }
        }
     }
}

How could I improve such code so it doesn't require all these if statements? They seem pretty messy.

\$\endgroup\$
2
  • \$\begingroup\$ I'd change your attr names if you can for readability \$\endgroup\$
    – Toby Allen
    Apr 28 '13 at 20:22
  • 2
    \$\begingroup\$ This post is off-topic for two reasons: it is unclear what this code is doing, and the code is hypothetical ("//perform action here") \$\endgroup\$
    – SirPython
    Sep 26 '15 at 21:28
6
\$\begingroup\$

Assuming the attribute names are truly attr1 through attr5, this is nice and terse.

$allEqual = true;
for ($i = 0; $i <= 5; $i++) {
    if ($player[$i]['value'] != $server->clean_attr($row['attr' . ($i + 1)]) {
        $allEqual = false;
        break;
    }
}

if ($allEqual) {
    // ...
}

It doesn't take much tweaking to have a more varied set of attribute names.

$attrs = array('one', 'two', 'three', 'four', 'five');
$allEqual = true;
for ($attrs as $player => $attr) {
    if ($player[$player]['value'] != $server->clean_attr($row[$attr]) {
        // ... the rest is identical ...
\$\endgroup\$
1
\$\begingroup\$

You could separate the array checks and the conditional: create a walker and store the result of a failed comparison in a separate container. Then check just that stored value:

Walker

function walk_players( &$player, $key, $context )
{
    $server   = $context['server'];
    $row_attr = $context['row'][ 'attr' . $key + 1 ];

    passed( $player[ $key ][ 'value'] === $server->clean_attr( $row_attr ) );
}

Storage

function passed( $test = NULL )
{
    static $pass = FALSE;

    if ( NULL === $test )
        return $pass;

    if ( FALSE === $test )
        $pass = $test;
}

Cleaned code

$context = array (
    'row'    => $row,
    'server' => $server
);

array_walk( $player, 'walk_players', $context );

if ( passed() )
{
    // do something
}
\$\endgroup\$
-1
\$\begingroup\$

Since PHP 5.5 you have the function array_column. This function could be used like this:

$values = arra_column($player,'value');

foreach($values as $i => $value){
       //stop checking further values
       if($value !== $server->clean_attr($row['attr'.$i+1]) return false;
       //perform action here
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Please explain how this recommendation is helpful to the user, and provide more information regarding the code that you have displayed. \$\endgroup\$
    – SirPython
    Sep 26 '15 at 22:55

Not the answer you're looking for? Browse other questions tagged or ask your own question.