0

There has got to be a better way of doing this. First of all pay no attention to the lack of validation and error checking. I just want the core of the script to work first. Also, Block 1 and Block 2 execute, Block 3 does not. Whats the best way to check for errors on MySQLi preperation AND executions?

Ultimately what we're doing below is
1) creating the account
2) Adding a notification to the account
3) Sending a welcome message to the user via the in-site mailbox.

    //REGISTER PROCESSING
    if(isset($_POST['email'], $_POST["password"], $_POST['username'], $_POST['register'])){

            $passwordHash = hash("sha1", $_POST["password"]);
            $time = date('H:i');
            $date = date("d/m/Y");
            $title = "Welcome to Study Bubble!";
            $content = "This is our demo training platform. It has limited functionality and is less than 30% through its development period.";
            $show = "1";
            $from = "Study Bubble";
            $message = "We thought we should send you a test message. We can use these to engage with users and users can use the system to contact eachother."; 

            //BLOCK 1                                                
            $stmt = $mysqli->prepare("INSERT INTO users (authuser,authpass,authemail) VALUES (?, ?, ?)");
            $stmt->bind_param('sss', $_POST['username'], $passwordHash, $_POST['email']);
            $result = $stmt->execute();

            if ($stmt->execute()) {
                //BLOCK 2
                $stmt = $mysqli->prepare("INSERT INTO notifications (user,date,time,title,content) VALUES (?, ?, ?, ?, ?)");
                $stmt->bind_param('sssss', $_POST['username'], $date, $time, $title, $content);
                $result = $stmt->execute();

                if ($stmt->execute()) {
                    //BLOCK3
                    $stmt = $mysqli->prepare("INSERT INTO messages (to,from,date,time,message,show) VALUES (?, ?, ?, ?, ?, ?)");
                    $stmt->bind_param('ssssss', $_POST['username'], $from, $date, $time, $message, $show);
                    $result = $stmt->execute();

                    if ($stmt->execute()) {

            $_SESSION['auth'] = $_POST['email'];
            header('Location: account-wizard.php');
                    }
                }   
            }
        }

So what I'm ultimately looking to learn here is:
1) Sensible way to check for errors in MySQLi (I'm googling it atm too) for debugging
2) Why block 3 isn't executing but block 1 and 2 is (number 1 should answer this for me) :D
3) A better way than nesting these: if ($stmt->execute()) { if one exists.

Thanks in advance people, really appriciated.

3
Contributors
8
Replies
26
Views
4 Years
Discussion Span
Last Post by mmcdonald
0

Perhaps this may help.

Cheers Pritaeas, I ran a search on Daniweb before posting this no idea why I couldn't find that. Good guide.

And recommendations on the nested if ($stmt->execute()) { or is that already the most efficient way?

0

Personally I wouldn't nest. I'd store the result and use sequential if's instead.

$result = $stmt->execute();
if ($result) {
    $result = $stmt->execute();
}
if ($result) {
    $result = $stmt->execute();
}

Better than that would be to use a transaction, so if one fails, they all fail.

0

I'd personally want to split those into functions, or better yet, class methods.

class RegistrationManager()
{
    private $mysqli;

    public function __construct(mysqli $dbLink) {
        $this->mysqli = $dbLink;
    }

    public function execute($username, $password, $email, $time, $date, $title, $content, $show, $from, $message)
    {
        $mysqli->begin_transaction();
        if ($this->createUser($username, $password, $email) &&
            $this->createNotification($username, $date, $time, $title, $content) &&
            $this->createUser($username, $from, $date, $time, $message, $show))
        {
            $mysqli->commit();
            return true;
        }
        else {
            $mysqli->rollback();
            return false;
        }
    }

    private function createUser($username, $password, $email) {
        $passwordHash = hash("sha1", $password);
        $stmt = $this->mysqli->prepare("INSERT INTO users (authuser,authpass,authemail) VALUES (?, ?, ?)");
        $stmt->bind_param('sss', $username, $passwordHash, $email);

        return $stmt->execute();
    }

    private function createNotification($username, $date, $time, $title, $content) {
        $stmt = $this->mysqli->prepare("INSERT INTO notifications (user,date,time,title,content) VALUES (?, ?, ?, ?, ?)");
        $stmt->bind_param('sssss', $username, $date, $time, $title, $content);

        return $stmt->execute();
    }

    private function createMessage($username, $from, $date, $time, $message, $show) {
        $stmt = $this->mysqli->prepare("INSERT INTO messages (to,from,date,time,message,show) VALUES (?, ?, ?, ?, ?, ?)");
        $stmt->bind_param('ssssss', $username, $from, $date, $time, $message, $show);

        return $stmt->execute();
    }
}

Which could then be used in the main code like:

$time = date('H:i');
$date = date("d/m/Y");
$title = "Welcome to Study Bubble!";
$content = "This is our demo training platform. It has limited functionality and is less than 30% through its development period.";
$show = "1";
$from = "Study Bubble";
$message = "We thought we should send you a test message. We can use these to engage with users and users can use the system to contact eachother."; 

$manager = new RegistrationManager($mysqli);
if ($manager->execute(
        $_POST["username"], $_POST["password"], $_POST["email"], 
        $time, $date, $title, $content, $show, $from, $message)) {
    $_SESSION['auth'] = $_POST['email'];
    header('Location: account-wizard.php');
}
else 
    $_SESSION["registration_error"] = true;
    header('Location: registration.php');
}
exit;

There is one other thing I'd like to point out. You seem to be storing the dates in two parts, both of which are strings. That's not ideal. You always want to try to use the database date/time types for dates. In MySQL, those are DATETIME, DATE, TIME and TIMESTAMP. In your case, you'd be better of using DATETIME. You can easily populate those types of fields from within MySQL itself:

INSERT INTO tbl VALUES(`when_created`) VALUES(NOW())

MySQL always returns date-time values in the string format: "YYYY-MM-DD HH:MM:SS", for example: 2013-07-15 11:45:30.

0

Cheers for the advice guys, really appricicated.

There is one other thing I'd like to point out. You seem to be storing the dates in two parts, both of which are strings. That's not ideal. You always want to try to use the database date/time types for dates. In MySQL, those are DATETIME, DATE, TIME and TIMESTAMP. In your case, you'd be better of using DATETIME. You can easily populate those types of fields from within MySQL itself:
INSERT INTO tbl VALUES(when_created) VALUES(NOW())
MySQL always returns date-time values in the string format: "YYYY-MM-DD HH:MM:SS", for example: 2013-07-15 11:45:30.

What if I want to call the date in one place and the time in another and they're both stored in one field? Would I just add SELECT LEFT('datetime', 10); and RIGHT('datetime', 4); to my queries?

0

OMG I'm such an idiot (I blame the tiredness) I'd just use: substr($datetime, -Y, -Z);.. sorry

2

You could also do it this in MySQL.

SELECT 
    DATE(`fieldname`) AS `date`, 
    TIME(`fieldname`) AS `time`
FROM the_table

People tend to want to customize the format of date though, which makes the PHP strtotime and date functions more useful.

$timestamp = strtotime($dateFieldValue);
$date = date("d/m/Y", $timestamp);
$time = date("H:i", $timestamp);

That can also be achieved in MySQL, using the DATE_FORMAT function, but doing it in PHP tends to make it more flexible; easier to configure. Especially once you start separating the business logic and database interaction more.

By the way, there are a lot of Date and Time Functions in MySQL, that deal mostly with the date-time types. That's one reason why you always want to use those types for dates.

Edited by Atli

This question has already been answered. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.