can i write this a better way?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
slash_gnr3k
Forum Commoner
Posts: 43
Joined: Tue Nov 28, 2006 6:41 pm

can i write this a better way?

Post 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!
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Post 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
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post 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
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post 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.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post 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.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

ole wrote:But you might like to know that isset() will take multiple parameters.
... Learn something new everyday.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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
}
?>
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post 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.
Post Reply