elseif conditional expression evaluates to TRUE when it should not?

[terungwa]

I thought this should be a very trivial issue but I was wrong.

I have a name filteration function below that checks user input for:

1. empty string,
2. regex pattern,
3. length of string

In using the function ro check user input, I am using the elseif logic function as shown below.

I think the 'elseif conditional expression evaluates to TRUE' ( returning my custom error code 100) , when it should not, or am I missing something??

function CheckFullname($string, $maxlen, $minlen)
{
   /*
     define error codes
     100 = wrong pattern match
     101 = string is too long
     102 = string is too short
     0 = empty input
    */

    // now we see if there is anything in the string
    if ($string = '')
    {
        return 0;
    }
// then we check fullname format
    elseif (!preg_match('/^(?:[\p{L}\p{Mn}\p{Pd}\'\x{2019}]+\s[\p{L}\p{Mn}\p{Pd}\'\x{2019}]+\s?)+$/u', $string))
    {
        return 100;
    }
    //how long the string is
    elseif (strlen($string) > $maxlen)
    {
        return 101;
    }
    // how short the string is
    elseif (strlen($string) < $minlen)
    {
        return 102;
    }
    else
    {
        // if all is not well, we return true
        return true;
    }
}
// this function executed below is returning error code 100.
$string = "John Doe";
if(Checkfullname($string, 40, 4)== 0)
{
    echo 'Your name is required.';
}
elseif(Checkfullname($string, 40, 4) == 100)
{
    echo 'Please provide your full name without any funny characters.';
}
elseif(Checkfullname($string, 40, 4)== 101)
{
    echo 'Your name should be at most 40 characters long.';
}
elseif(Checkfullname($string, 40, 4)== 102)
{
    echo 'Your name should be at least 4 characters long.';
}
else
{
    $fullname = safe($string);
}

[requinix]

if ($string = '')

[terungwa]

if ($string = '')

Yep I get it, the ' if ' check was redundant as the regex was already doing that right?

I eliminated that and the filter function is validating as expected.

 

Thanks for the heads up requinix.

[requinix]

It's not redundant because you needed to distinguish between an empty string (0) and input that fails the pattern constraint (100).

 

I was pointing out the = single equals sign. That means assignment, even inside an if condition, so when the next bit of code executes (the regex check) $string will always be "". It should be a == comparison.

[Psycho]

EDIT:

 

Also your usage of the function is redundant:

 

// this function executed below is returning error code 100.
$string = "John Doe";
if(Checkfullname($string, 40, 4)== 0)
{
    echo 'Your name is required.';
}
elseif(Checkfullname($string, 40, 4) == 100)
{
    echo 'Please provide your full name without any funny characters.';
}
elseif(Checkfullname($string, 40, 4)== 101)
{
   . . .

 

Call the function ONE TIME. Then perform whatever actions on that return value. As you have it now you could introduce a bug by accidnetally changing one of the parameters in a function call on one line and not another.

Edited October 1, 2014 by Psycho

[Psycho]

A couple other things:

 

1. You should trim() the string.

2. You don't need to use a string of if/else statements since if any are true they will do a 'return' and exit the function

3. You can use a switch() to check the return value

4. Be careful of using '0' as a return value when many values can be returned because 0 can also be interpreted as the Boolean false.

5. Think about the order in which you perform the checks since a value can actually meet multiple error conditions. I would do min/max lengths before checking the format.

 

Here are some changes in the logic that I think work better

 

function CheckFullname($string, $minlen, $maxlen)
{
   /*
     define error codes
     100 = wrong pattern match
     101 = string is too long
     102 = string is too short
     0 = empty input
    */
 
    //Trim the string
    $string = trim($string);
 
    //Check for empty value
    if(!strlen($string))
    {
        return 0;
    }
    //Check minimum length
    if(strlen($string) < $minlen)
    {
        return 102;
    }
    //Check maximum length
    if (strlen($string) > $maxlen)
    {
        return 101;
    }
    //Check fullname format
    if (!preg_match('/^(?:[\p{L}\p{Mn}\p{Pd}\'\x{2019}]+\s[\p{L}\p{Mn}\p{Pd}\'\x{2019}]+\s?)+$/u', $string))
    {
        return 100;
    }
 
    // No errors detected
    return true;
}
// this function executed below is returning error code 100.
$fullName = "John Doe";
$nameMinimum = 4;
$nameMaximum = 40;
 
$nameError = false;
switch(Checkfullname($fullName, $nameMinimum, $nameMaximum))
{
    case 0:
        $nameError = 'Your name is required.';
        break;
    case 100:
        $nameError = 'Please provide your full name without any funny characters.';
        break;
    case 101:
        $nameError = 'Your name should be at most 40 characters long.';
        break;
    case 102:
        $nameError = 'Your name should be at least 4 characters long.';
        break;
}
 
if($nameError)
{
    echo $nameError;
}
else
{
    $fullname = safe($string);
}