PHP function returns false even if executed successfully

Question

I have the following functions.

function generateOTP($theUser, $thePhone){
  global $pdo;

  $stmt = $pdo->prepare("DELETE FROM otp WHERE phone = :phone");
  $stmt-> bindValue(':phone', $thePhone);
  $stmt-> execute();

  $stmt = $pdo->prepare("INSERT INTO otp(phone, otp, type, validity)VALUES(:phone, :otp, :type, :val)");
  $stmt-> bindValue(':phone', $thePhone);
  $stmt-> bindValue(':otp', rand(1000, 9999));
  $stmt-> bindValue(':type', 'new');
  $stmt-> bindValue(':val', date('Y-m-d H:i:s', strtotime('+5 mins')));
  $stmt-> execute();
}

function sendOTP($theUser, $thePhone){
  global $pdo;
  generateOTP($theUser, $thePhone);
}

// CALLED SOMEWHERE LIKE THIS
if(sendOTP($theUser, $thePhone){
  echo "OTP SENT";
}else{
  echo "OTP SENDING FAILED";
}

The problem I have encountered is that even if the sendOTP() function is executed well and all the records have been inserted in the database, it always moves to the else block and prints OTP SENDING FAILED. In other words, it always returns false assuming that the function failed to execute successfully. But actually, function has been executed well and all the queries are executed properly. This is a strange issue I have never come across before. How can I solve this?

Your function doesn't return anything, so I'm not sure how you're expecting to be able to tell the real result. Trying to get the result of a function with no return statement will always result in null, which is false-y, which is why your if behaves as it does. — ADyson
– ADyson, Commented Jul 19, 2021 at 15:49
If you genuinely want to know whether all the PDO code succeeded then enable PDO error handling: php.net/manual/en/pdo.error-handling.php . You could then wrap it in a try/catch if you need to handle those. — ADyson
– ADyson, Commented Jul 19, 2021 at 15:55
Also your output messages are misleading (unless you have omitted some of the code) because all your code does is delete and insert values into a database. It doesn't actually send the value to anyone's phone. Sending could still easily fail after the insert has occurred, but I assume that process is handled elsewhere in your application (or in some code ommitted from the sendOTP() snippet in your example above). — ADyson
– ADyson, Commented Jul 19, 2021 at 15:57
@ADyson you are right and I even tried returning true as if(generateOTP($theUser, $thePhone){ return true; } but still the result is same. After trying every possible methods I knew, I was forced to raise a question. — Relaxing Music
– Relaxing Music, Commented Jul 19, 2021 at 15:58
@ADyson I know the PDO code executed well as the records are inserted in the database. Do I really still need to use the try catch method? — Relaxing Music
– Relaxing Music, Commented Jul 19, 2021 at 15:59

Slava Rozhnev · Accepted Answer · 2021-07-19 16:24:27Z

2

I can advice next solution (The otp table must have unique/primary key phone):

<?php
function generateOTP($theUser, $thePhone){
    global $pdo;
    try {
        $stmt = $pdo->prepare("
            INSERT INTO otp(phone, otp, type, validity) 
            VALUES (:phone, :otp, :type, :val)
            ON DUPLICATE KEY UPDATE
                otp = :otp,
                type = :type,
                validity = :val");
        $stmt-> bindValue(':phone', $thePhone);
        $stmt-> bindValue(':otp', rand(1000, 9999));
        $stmt-> bindValue(':type', 'new');
        $stmt-> bindValue(':val', date('Y-m-d H:i:s', strtotime('+5 mins')));
        return $stmt-> execute();
    } catch (PDOException $Exception ) {
        // print_r($Exception);
        // here you can log Exception
        return false;
    }
}

function sendOTP($theUser, $thePhone){
  global $pdo;
  return generateOTP($theUser, $thePhone);
}

Here You can test PHP code online

answered Jul 19, 2021 at 16:24

Slava Rozhnev

10.2k6 gold badges27 silver badges43 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Relaxing Music Over a year ago

Great.. Thank you :)

Omar Khaled · Accepted Answer · 2021-07-19 15:59:32Z

0

your functions doesn't return any values you need to change your code from :

function generateOTP($theUser, $thePhone){
  global $pdo;

  $stmt = $pdo->prepare("DELETE FROM otp WHERE phone = :phone");
  $stmt-> bindValue(':phone', $thePhone);
  $stmt-> execute();

  $stmt = $pdo->prepare("INSERT INTO otp(phone, otp, type, validity)VALUES(:phone, :otp, :type, :val)");
  $stmt-> bindValue(':phone', $thePhone);
  $stmt-> bindValue(':otp', rand(1000, 9999));
  $stmt-> bindValue(':type', 'new');
  $stmt-> bindValue(':val', date('Y-m-d H:i:s', strtotime('+5 mins')));
  $stmt-> execute();
}

function sendOTP($theUser, $thePhone){
  global $pdo;
  generateOTP($theUser, $thePhone);
}

to:

function generateOTP($theUser, $thePhone){
  global $pdo;

  $stmt = $pdo->prepare("DELETE FROM otp WHERE phone = :phone");
  $stmt-> bindValue(':phone', $thePhone);
  $stmt-> execute();

  $stmt = $pdo->prepare("INSERT INTO otp(phone, otp, type, validity)VALUES(:phone, :otp, :type, :val)");
  $stmt-> bindValue(':phone', $thePhone);
  $stmt-> bindValue(':otp', rand(1000, 9999));
  $stmt-> bindValue(':type', 'new');
  $stmt-> bindValue(':val', date('Y-m-d H:i:s', strtotime('+5 mins')));
  if ($stmt-> execute()) {
   return true;
  } else {
   return false;
  }
}

function sendOTP($theUser, $thePhone){
  global $pdo;
  if (generateOTP($theUser, $thePhone)) { 
   return true;
  } else {
   return false;
  }
}

now you made your function return true/false

answered Jul 19, 2021 at 15:59

Omar Khaled

661 silver badge9 bronze badges

14 Comments

ADyson Over a year ago

There are a lot more ways for the PDO code to fail than just the final execute() statement. PDO error handling enabled, possibly with a try/catch, would be a much more robust approach

Relaxing Music Over a year ago

Oh I got it now.. I had only tried to return true in the sendOTP() function. I needed to do that in both the functions. Got it. Also, else{ return false; } is not required. I can check with just true and see if(sendOTP($theUser, $thePhone)) which means if it's true else it will be default always mean false.

ADyson Over a year ago

@RelaxingMusic Be aware that if your DELETE fails to execute(), or any of the prepare() or bindValue() calls fail for any reason, this half-baked answer you've accepted would not detect it properly.

Relaxing Music Over a year ago

@ADyson sure.. I will keep that in mind :)

Relaxing Music Over a year ago

@ADyson or, can you please give a full baked answer to make it more secure?

|

Collectives™ on Stack Overflow

PHP function returns false even if executed successfully

2 Answers 2

1 Comment

14 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

14 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related