"Better" way to write this?

Question

Is there a better (ie, more readable) way to write this?

if (isset($input_vars['directive']) && $input_vars['directive'] == 'edit') {

Answer 1

Not really, unfortunately. You could wrap this code in a function and simply call that every time you need this.

function compareArrayItem($array, $key, $value) {
    return isset($array[$key]) && $array[$key] == $value;
}

if (compareArrayItem($input_vars, 'directive', 'edit')) {
    // Do something
}

But that seems kind of pointless to me (and less readable than your original code). Or you could lower the error reporting level to not include E_NOTICE so that you don't need the first expression at all.

error_reporting(E_ALL ^ E_NOTICE);

if ($input_vars['directive'] == 'edit') //...

But I wouldn't recommend doing this just for the sake of shortening your code.

If I were you, I'd just leave it alone. It's fine as-is.

Answer 2

The following would yield the same result every time.

if($input_vars['directive'] == 'edit'){

This is because if its not set then its not 'edit', if its 'edit' then its set.

This does return a notice but you may turn it that feature off (Not saying that you should) from your PHP installation.

Answer 3

I'd say:

if(@$input_vars['directive']=='edit') {

Would be slightly more readable and it doesn't produce warnings if directive isn't there.

Answer 4

I'm going to guess that you'll be testing $input_vars['directive'] against more than one value (otherwise, why would you not just have a simple boolean stored in $input_vars['edit'] or similar?). I would also hazard a guess that you're doing those tests one after the other (if 'edit' do X, else if 'display' do Y).

In such a case, just put the isset() test in an if statement and nest the others inside that (switch/case flow wouldn't be a bad choice).

Answer 5

If the set of allowed values in $input_vars is known and the checks you mention are all over the place the following helper function will do:

function item ($array, $key) {
    if (isset ($array [$key]))
        return $array [$key];
    else
        return NULL; // Or use whatever is more appropriate
}

Then the original code can be changed to look like

 if (item ($input_vars, 'directive') == 'edit') { ...

This is not only more readable, but also removes duplication: both array variable and key appear only once.