Insert into two tables during user registration using PHP

Question

I'm working on creating a user registration script in PHP. I have two tables that need to be inserted into. One that stores some general user information, and another that stores their login credentials. My issue is that my code to perform these insert is a mess of if/else blocks and I'm trying to figure out a good way to condense this down into a logical and clean code block.

Note that for now I have just placed some echo's that will later be replaced by a real error handling system once I truly know how many paths I'm going to need.

Can someone please review this script and let me know some improvements I can make to it? I know it's quite ugly right now.

$mysqli = getMysqlConnection(); if ($stmt = $mysqli->prepare("INSERT INTO users (username, email, regtime, emailverified, type) values (?,?,?,?,?);")) { date_default_timezone_set("America/New_York"); $dateStr = date("m-d-Y h:i:s"); $emailverified = 0; $type = 0; $rc = $stmt->bind_param('sssii', $username, $email, $dateStr, $emailverified, $type); if($rc === true){ $rc = $stmt->execute(); if($rc === true) { if ($stmt = $mysqli->prepare("INSERT INTO usercreds (username, hash) VALUES (?, ?);")) { $rc = $stmt->bind_param('ss', $username, $hash); if($rc === true){ $rc = $stmt->execute(); if($rc !== true) { echo "so close..."; } } else { echo "well, shit..."; } } else { { echo "whoops..."; } } } else { echo "noooooooooo!"; } } } else { echo "uh oh"; } echo "done"; 

Answer 1

You should return early to reduce nesting and make it more obvious when what values are returned:

if (!$stmt = $mysqli->prepare("INSERT INTO users (username, email, regtime, emailverified, type) values (?,?,?,?,?);")) { echo "uh oh"; } [...] if($stmt->bind_param('sssii', $username, $email, $dateStr, $emailverified, $type) !== true){ // you actually didn't have an else for this } [etc] 

You should also replace your echoes with exceptions with proper error messages.

Misc

• your indentation is off, and you should use a minimum of 4 spaces.
• rc is not a good variable name, and you shouldn't reuse variables for different things.
• you should also extract this into two functions: insertUser($username, $email, ...) and insertUserCredentials($username, $hash).

Answer 2

Briefly, I would at least think about organisation ...

<?php date_default_timezone_set("America/New_York"); function updateUser($connection, $username, $email, $regTime, $isVerified, $type) { $stmt = $connection->prepare("INSERT INTO users (username, email, regtime, emailverified, type) values (?,?,?,?,?);"); $rc = $stmt->bind_param('sssii', $username, $email, $dateStr, $emailverified, $type); $rc = $stmt->execute(); return $rc ? true : false; } function updateCredentials($username, $hash) { $stmt = $mysqli->prepare("INSERT INTO usercreds (username, hash) VALUES (?, ?);"); $rc = $stmt->bind_param('ss', $username, $hash); $rc = $stmt->execute(); return $rc ? true : false; } $username = 'someUserName'; $dateStr = date("m-d-Y h:i:s"); $emailverified = 0; $type = 0; $mysqli = getMysqlConnection(); $updatedUser = updateUser($mysqli, $username, $email, $dateStr, $emailVerified, $type); if (!$updatedUser) { throw new \Exception("There was a problem updating the user:" . $username); } $updatedCreds = updateCredentials($mysqli, $hash); if (!$updatedUser) { throw new \Exception("There was a problem updating the credentials:" . $username); } echo "done";