Caution - noob alert. Gotta start somewhere...

1) Currently following a couple tutorials and simply can't locate the error preventing the creation of the table. I've even done this before. So simple! Yes, I could manually add the table but I'd rather learn this way.

2) How do I create another table with this script? Can I simply use "$sql2;" and change the conditional to something like if ($sql & $sql2 === TRUE)??

Working on the first version of an upvote/downvote feature.

Thanks!!

<?php 

$con = mysql_connect("localhost", "root", ""); 

if (!$con) { 

die("Cannot connect:" . mysql_error());

}

if (mysql_query("CREATE DATABASE vote", $con)) {

echo "Database created.";

} else {

echo "Error: " . mysql_error();

}

mysql_select_db("vote", $con);

$sql = "CREATE TABLE messages (
mes_id INT PRIMARY KEY AUTO_INCREMENT,
mes TEXT,
up INT,
down INT)";

mysql_query($sql, $con);

if ($sql === TRUE) {

    print "<br /><br />Success in TABLE creation! Happy Coding!";

} else {

    print "<br /><br />No TABLE created. Check";
} 

mysql_close($con); 

?>
share|improve this question

closed as off topic by Anthony Pegram, mseancole, palacsint, Winston Ewert Jun 9 '12 at 3:51

Questions on Code Review - Stack Exchange are expected to relate to code review request within the scope defined in the FAQ. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about closed questions here.

1 Answer

I'm not quite sure if this question is on topic as it's asking how to fix code rather than reviewing existing code, however, I do have a few review related suggestions:


MySQL Extension vs PDO

Consider using PDO. The mysql_* functions are thin wrappers around the C API, and PDO offers a lot more functionality with a lot cleaner interface. For example, PDO offers prepared statements and clean transaction handling that is not tied to SQL statements, but can rather be begun, committed or aborted programmatically.

Another advantage is exception throwing (though really whether that's an advantage or not is opinion).

Instead of explicitly checking the return value of every statement, it's convenient to know that an exception will be thrown:

$db = new PDO($dsn, $username, $password);
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

I've found that this allows very flexible errors handling. Most of my applications are coded so that if an exception bubbles up to the top, the user will see a fairly pretty error message while the exception is also logged into the error log. This means that I have to go through fairly minimal effort to present errors to users in a view similar to that of the rest of the site. This is by no means impossible without exceptions, but it's easier (in my opinion).

Of course, the option to handle an error at the point of erroring is still there:

try {
    $db->exec("CREATE TABLE ... ");
} catch (PDOException $e) {
    echo 'Uh oh! Something went wrong!';
}

Without writing a blog post, it's pretty hard to convince someone of the benefits of PDO. In fact, for very small applications, there might not be any benefits. However, I've found in my years of PHP programming that PDO has provided a much better experience than the mysql_* family.


Pure SQL instead of PHP

This is mostly an opinion rather than anything I can backup, but I tend to dislike PHP based installation scripts for database schema. Unless you're using PHP to do complex actions like conditionally alter schema or check for the current state of the database, you might as well use pure SQL. In other words, with your current PHP script, there's no reason to use that instead of just a .sql file.

mysql -u 'user' -p 'password' database_name < schema.sql

Is pretty simple. (Or even copy/pasting into PHPMyAdmin)

What I tend to do is to create my initial schema, and then once that has been released (i.e. it's been installed in one or more locations), I never change it. Instead, I create update scripts that do what I want. That way, keeping schema controlled is fairly simple. (Though this does require somehow knowing where each application's schema is in terms of revisions -- for this reason I try to make things as idempotent as possible.)


Errors

Anyway, while I'm at it, here's what's wrong with your code:

if ($sql === TRUE) {

$sql is a string. In particular, SQL is your table creation string. In other words, you're not checking if the statement executed successfully, you're checking the statement.

It's basically like doing:

$string = "Hello, my name is Corbin.";
if ($string === true) { /* never true */ }

A string can never be === to true (or false for that matter), so even if the table is being created, it's going to go to the else branch.

The proper way would be:

if (mysql_query($sql, $con)) { /* success */ }

Or:

$created = mysql_query($sql, $con);
if ($created) { }

In this case, mysql_query will return a boolean, so you could do:

if ($created === true) {}

However, don't get into a habit of this, as mysql_query returns a resource for queries that return resultsets (select/show/etc).

share|improve this answer
Thanks so much!! – incagneato Jun 8 '12 at 21:54
@incagneato No problem! – Corbin Jun 8 '12 at 22:57

Not the answer you're looking for? Browse other questions tagged or ask your own question.