user2284433
user2284433

Reputation:

Not returning value of PHP function correctly?

I'm using a function to validate each part of a form. For this particular form, the phone number is an optional entry. So I came up with the following function:

if(isset($_POST['phone'])) { $phone = $_POST['phone']; }

//Call phone validation function
$phone = validPhone($phone);
if (strlen($phone) == 0) {
    $mistakes[] = 'Your phone number must contain only NUMERIC characters.';
}

    function validPhone($phone) {
        if ($phone != '') {
            $phone = trim($phone);
            if (!ctype_digit(str_replace(' ', '', $phone))) {
                $phone = '';
            } else {
                //accept phone entry and sanitize it
                $phone = mysql_real_escape_string(stripslashes($phone));
            }
        } else {
            $phone = 'not specified';
        }
        return $phone;
    }

My issue is that when you don't enter something for $phone, this bit:

else {
    $phone = 'not specified';
}

seems to be skipped, as there is nothing entered into the database.

The following is the appropriate database PHP:

//Insert Into Database
$sql="INSERT INTO signups (phone)
VALUES
(''".$phone."')";

if (!mysql_query($sql)) {
  die('Error: ' . mysql_error());
}

My intention is for whenever nothing is entered by the user, the entry to the database would be 'not specified'. Am I doing something incorrectly?

Upvotes: 0

Views: 125

Answers (3)

Mike Brant
Mike Brant

Reputation: 71384

The problem arise because of the loose comparator ==.

If $phone = null (like if the value wasn't posted), $phone == '' evaluates as true. Because '' is loosely equivalent to null. You either nee to use a strict comparison ===, or better yet use empty(). I personally handle all such stuff up front when initializing the variable (including sanitizing the input value, as there is no sense in passing unsafe data further into your application. Here is what I might suggest:

$phone = '';
if (!empty($_POST['phone'])) {
    $phone = mysql_real_escape_string(stripslashes(trim($_POST['phone'])));
}

$phone = validPhone($phone);
if ($phone === '') {
    $mistakes[] = 'Your phone number must contain only NUMERIC characters.';
}

function validPhone($phone) {
    if ($phone === '') {
        return 'not specified';
    } else if (!ctype_digit(str_replace(' ', '', $phone))) {
        return '';
    } else {
        return $phone;
    }
}

Also you should really look into replacing the deprecated mysql_* functions in your code with mysqli or PDO equivalents. Note you also need to change your code to populate $mistake, as with a returned value of 'not specified' for $phone, you would not trigger that conditional.

Upvotes: 0

David
David

Reputation: 18271

if (!empty(trim($phone))

would match is $phone = 0, "", null, false so it would be a catch all

More importantly, you should look into doing test driven development as it will improve your development speed and cut down on errors. Checkout PHPUnit - https://github.com/sebastianbergmann/phpunit/

Upvotes: 2

raidenace
raidenace

Reputation: 12826

Change if ($phone != '') { to if (trim($phone) != '') {

Upvotes: 0

Related Questions