php and mysql, best practices

Question

I started working with php and mysql today. Basically, what I have, is an empty page with pieces that I fill in from looking up an id in a database. So on my home page I have an url that looks like this:

<a href="content/display.php?id=id1">

And then in my display.php I have this:

<?php
    include '../includes/header.php';
    $id = $_GET['id'];
    $mysqli = new mysqli('localhost','username','password','dbname');
    if($result = $mysqli->query("SELECT * FROM portfolio WHERE id='".$id."'"))
    {
        while($row = $result->fetch_object())
        {
            $head = $row->head;
            $img1 = $row->img1;
            $img2 = $row->img2;
            $img_url = $row->imgurl;
            $img_thumb = $row->imgthumb;
            $vid = $row->vid;
            $swf = $row->swf;
            $url = $row->url;
            $url_text = $row->urltext;
            $text = $row->text;
        }
    }
    else echo $mysqli->error;
?>

It's a sparse table in that not all of those fields will have information (many might be null). Basically they contains file names and then in the html I have code that looks like this:

if(isset($img1))
                    {
                        echo '<img src="images/'.$img1.'" />';
                    }

A couple of questions,

1. Is this the best way to do this?
2. Everytime I visit display.php, I am reopening a database connection right? That can't be good...
3. I chose to put the names of the files in the database, rather than entire path names, or even the actual files themselves, figuring that, if I change the name of the file I can go into the database and update it for the file I want to change. If I change the path, I can just change it once in the html. Is that the best idea?

Thanks!

Answer 1

You are vulnerable to SQL injection, properly type cast your variables:

$id = (int) $_GET['id'];

Use functions such as mysql_real_escape_string or even better use:

• Prepared Statements

Answer 2

1) No, although that's the easiest way for beginning. After you feel comfortable with basics, you should spend some time considering different approaches to application structure. Most important rule is to separate concerns. Don't mix database code with business logic code with presentation code. But like I said, it's not something you should worry about on your first day. For now just learn basics.

2) There's no other way actually. For a web application each request from browser is like an individual run of application. There is a possibility to use so called persistent database connections, but just like in previous point, that's something you should not deal with on your first day, as they require specific configuration of your web server. For the time being just use normal connections.

3) That's pretty sensible idea. You could also define your image path as a PHP constant, so that in case a change is needed, you only change this one constant.

4) What sAc says in his answer is very important. Read about SQL injections and how to prevent them.

Answer 3

SQL injection & prepared statements are already mentioned. An addition to that would be:

else echo $mysqli->error;

Change that to:

else trigger_error($mysqli->error,E_USER_ERROR);

Why you ask? Because visitors should have no idea about your database, and cannot fix the error, so they plain shouldn't see it. This way, you can safely develop with display_errors on, and on the live site display_errors is off, and you log_errors in an error log.

Answer 4

Looks like you have good handle on what you want to do. I don't know how much development background you have, but it would be a good idea to start learning about MVC's in php like CakePHP, Fuse, or even Zend Framework(bleh!!!). I'll save you time on more robust applications by pre defining all your basic db interface, template handling, session handling, and let you worry about higher level problems, like what's for lunch! :)