Your PDO usage is fine (ish).
However, your code structure is a mess. The most obvious thing is that jobs SQL request. Surely you’ll have multiple entry points to insert a new job to your database. Imagine that your query string changes - do you really want to have to go through your code and replace every occurence + every single SELECT that either looks directly at it or JOINs it?
Secondly, you want to use a way to pass $odbc around, preferably one that does not rely on a global variable (so it cannot be overwritten by accident).
Thirdly, you’re falling victim to namespace pollution. All your variables are global - this is a sign of failure in scoping.
Here is a re-written version of your code, which might make more sense:
DB class
[php]<?php
class DB {
private static $instance;
protected $PDO;
private static $username = “user”;
private static $password = “pass”;
private static $db = “dbname”;
private function __construct() {
$this->PDO = new PDO(“mysql:host=localhost;dbname=”.static::$db,static::$username,static::$password);
// Blah
}
public static function config(stdClass $cfg) {
foreach ($cfg as $k => $v) {
if (isset(static::${$k})) static::${$k} = $v;
}
}
public static function getInstance() {
if (!static::$instance) static::$instance = new self();
return static::$instance;
}
public function run($q) {
$r = $this->PDO->prepare($q);
// This function allows you to also deal with performance tracking, logging etc.
return $r;
}
}
[/php]
Jobforms model
[php]<?php
class Jobforms {
public static function find($id) {
$DB = DB::getInstance();
$q = $DB->run(“SELECT * FROM jobform WHERE id=:id”);
$r = $q->execute(array(":id" => $id));
return $r;
}
public static function create(stdClass $params) {
$DB = DB::getInstance();
$q = $DB->run(“INSERT INTO jobform (firstname, lastname, email, phone) VALUES (:fn, :ln, :email, :phone)”);
foreach ($params as $k => $v) {
$q->bindValue($k,$v);
}
return $q->execute();
}
}[/php]
This allows you to condense your code down to:
[php]<?php
if ($_SERVER[‘REQUEST_METHOD’] !== “POST”) throw new Exception(“Wrong request type”);
if (!isset($_POST[‘firname’) || !isset($_POST[‘lastname’]) || !isset($_POST[‘email’]) || !isset($_POST[‘phone’])) throw new Exception(“Some fields were not found”);
if (!filter_var($_POST[‘email’], FILTER_VALIDATE_EMAIL)) throw new Exception(“Incorrect email”);
$v = new stdClass();
$v[":fn"] = $_POST[‘firstname’];
$v[":ln"] = $_POST[‘lastname’];
// More assignments
Jobforms::create($v);[/php]
Which is not only a lot neater, but also a lot more reusable. Note that it needs a try/catch block.
What I have just pointed you towards is called ORM, by the way. It stands for Object-Relations Mapping, and it is commonly referred to as using “models”. A model is an object instance that represents a set of related data entries of a certain type in your database. Some frameworks and libraries do this exceedingly well - look up Doctrine!
