Clumsy PHP5 code reading from an SQLite3 database Some time ago I wrote myself a simple blog. The index page is generated
with PHP5, using an SQLite3 database via the variable id handed to it via
the URL.

The SQLite database contains a table called pages. The id refers of course
to the id intergers in this table. Other columns are title, body, time and
timezone.

The title and body are text as you'd expect, the timezone is usually
"Asia/Tokyo", and the time is seconds from the epoch (I just use date +%s,
when I add an entry, nothing fancy).

I use the following code to get these variables from a table, and fill in
a generic page with them.

&lt;?php
    // Open the database.
    $db = new PDO("sqlite:pagesdatabase.db");
    // Is id set? If not get id of last page and put it in $id.
    if (!isset($_GET["id"])) {
        foreach ($db-&gt;query("select max(id) from pages") as $lastid) {
            $id = $lastid[0];
        }
    }
    // If id is set, get the value and put it in $id.
    else {
        $id = $_GET['id'];
    }
    // Using the id number, grab the title, body, time and timezone of post.
    foreach ($db-&gt;query("select * from pages where id = $id") as $page) {
        $title = $page['title'];
        $body = $page['body'];
        $time = $page['time'];
        date_default_timezone_set($page['timezone']);
    }
?&gt;


This code works, but seems rather clumsy to me. For example, foreach
appears twice, but shouldn't be necessary. To be honest, it's been a while
since I wrote this code, and for some reason I had trouble using sqlite_open
and other more obvious functions. Can someone help me write the above in a more
terse way?
You are right, the foreaches can be eliminated.  The first foreach which grabs the highest id, can be eliminated by taking advantage of the fact that the query will only ever return a single cell and using the PDOStatement::fetchColumn() method.  Since page ids are unique, the second foreach can be removed because the second query will only ever return a single row.

An additional problem is that you are not sanitizing user input in your SQL.  This is easy to fix with PDO by using a parameterized statement:

&lt;?php
// Open the database.
$db = new PDO("sqlite:pagesdatabase.db");

// Is id set? If not get id of last page and put it in $id.
if (!isset($_GET["id"])) {
    $id = $db-&gt;query("select max(id) from pages")-&gt;fetchColumn();
}
// If id is set, get the value and put it in $id.
else {
    $id = $_GET['id'];
}

// You may consider doing an additional check here in case there are no pages, in which
// case id will be null
if ($id === null) {
    // ...
    exit;
}

// Using the id number, grab the title, body, time and timezone of post.
$page = $db-&gt;prepare("select * from pages where id = :id")
           -&gt;execute(array('id' =&gt; $id))
           -&gt;fetch();

// A check for an invalid id should be made here
if ($page === null) {
    // ...
    exit;
}

$title = $page['title'];
$body = $page['body'];
$time = $page['time'];
date_default_timezone_set($page['timezone']);
?&gt;


I have only quickly prepared this answer from experience and have not actually run the code above.  It should be enough to give you an idea but may have syntax errors.  If it does, comment on the post and I will fix it.  Also, if reducing this code to the fewest lines possible is your goal, setting $id can be accomplished with a ternary and the actual retrieval of the page along with the error checks can be encapsulated in a function or a class.

&lt;?php
// Open the database.
$db = new PDO("sqlite:pagesdatabase.db");

// Determine if a page id is provided, if not use the most recent page
$id = isset($_GET['id'])
  ? $_GET['id']
  : $db-&gt;query("select max(id) from pages")-&gt;fetchColumn();

try {
    $page = Page::fetch($id, $db);

    $title = $page-&gt;getTitle();
    $body = $page-&gt;getBody();
    $time = $page-&gt;getTime();
    date_default_timezone_set($page-&gt;getTimezone());

} catch (Exception $e) {
    // This exception should be thrown by Page::fetch if no pages exist or an invalid
    // id is given
    // ...
    exit;
}
?&gt;


If this is preferred I will leave the class definition to you.