Page 1 of 1

can i write this a better way?

Posted: Wed Jan 31, 2007 3:51 pm
by slash_gnr3k
i have the following condition in php. just wondering if there is a better, tidier way of writing it...

Code: Select all

if(isset($_POST['newdishname'])  || isset($_POST['newmethod']) || isset($_POST['newpreptime']) || isset($_POST['newcooktime']) || isset($_POST['newvegetarian']) || isset($_POST['newtype']))
		{
.....

}
thanks!

Posted: Wed Jan 31, 2007 3:56 pm
by shiznatix

Code: Select all

$needed_arr = array(
  'newdishname',
  'newmethod',
  'newpreptime',
  'newcooktime',
  'newvegetarian',
  'newtype',
);

$found = false;

foreach ($_POST as $key => $val)
{
    if (in_array($key, $needed_arr))
    {
        $found = true;
        continue;
    }
}

if ($found)
{
    //.....
}
thats one way thats a little cleaner or at least easier to follow. I am sure there is a better, more clever way to do it though

Posted: Wed Jan 31, 2007 4:24 pm
by Mordred
The other way around, loop over $needed_arr and check isset($_POST[...]) and use break instead of continue, there's a subtle difference :P

Posted: Wed Jan 31, 2007 4:38 pm
by superdezign
It's only really subtle (to the efficiency, not the code, of course) if one of the later post variables are the first ones to be found as unset.

Posted: Wed Jan 31, 2007 6:14 pm
by RobertGonzalez

Code: Select all

<?php
$checks = array('newdishname', 'newmethod', 'newpreptime', 'newcooktime', 'newvegetarian', 'newtype');

foreach ($checks as $check)
{
  if (isset($_POST[$check]))
  {
    break;
  }
}
?>
A little bit of the both of your suggestions.

Posted: Wed Jan 31, 2007 7:59 pm
by Ollie Saunders
Inspired from Everah's:

Code: Select all

function issetMultiKeys($source, $keys)
{
    foreach ((array)$keys as $key) {
        if (!isset($source[$key])) {
            return false;
        }
    }
    return true;
}
I generally like to post functional rather than non-functional code where functions can be identified.

But you might like to know that isset() will take multiple parameters.

Posted: Wed Jan 31, 2007 10:15 pm
by superdezign
ole wrote:But you might like to know that isset() will take multiple parameters.
... Learn something new everyday.

Posted: Thu Feb 01, 2007 12:12 am
by RobertGonzalez
Oooh, speaking of functions...

Code: Select all

<?php
/**
 * Checks whether an index is set on an array member.
 * 
 * This function will tell whether a particular array var 
 * is set. It can be used with to check a single string 
 * array index, a single numeric index or multiple 
 * indeces of an array. If the optional $requireall 
 * flag is set to true, it will make sure all members 
 * of the $required array are set in the $checkthis array.
 * @access public
 * @param mixed $required The value to search for isset in $checkthis
 * @param array $checkthis The array to check the set values if
 * @param boolean $requireall Optional flag to force all $required field be set
 * @return boolean True if all criteria is met, false otherwise
 */             
function is_value_set($required, $checkthis, $requireall = false)
{
    // If the index or array to check are empty, return false
    if (empty($required) || empty($checkthis))
    {
        return false;
    }

    // If the required value to check is a string or number
    if (is_string($required) || is_numeric($required))
    {
        if (isset($checkthis[$required]))
        {
            return true;
        }

        return false;
    }

    // If the required value is an array
    if (is_array($required))
    {
        // Count holder for requireall
        $checkcount = 0;
        
        // Loop through the $required value
        foreach ($required as $require)
        {
            if (isset($checkthis[$check]))
            {
                // If we don't require all and $required is set, return
                if (!$requireall)
                {
                    return true;
                }
                else
                {
                    // Increment the check counter
                    $checkcount++;
                }
            }
        }
        
        // If we are here then we check if the counts for required match
        if ($checkcount == count($required))
        {
            return true;
        }
        
        // The needed criteria were not met, return false 
        return false;
    }
    
    // Again, necessary criteria was not met, retun false
    return false;
}

// Set an array of required values
$checks = array('newdishname', 'newmethod', 'newpreptime', 'newcooktime', 'newvegetarian', 'newtype');

// Run them through the function 
if (!is_value_set($checks, $_POST))
{
    // Error out
}
?>

Posted: Thu Feb 01, 2007 5:00 am
by Ollie Saunders
Whoa my god! I'd prefer several smaller functions personally I think. Yep that would seem to be an example of the "Long Method" design smell for which the extract method refactor is recommended.