Reusable project for CRUD

Question

I wrote this class to make it reusable in any project. It uses PDO as the MySQL connection. Beside not using Setters and Getters, is this code secure and good practice for CRUD?

class ContactsRow
{
const PDO_CHARSET = PDO_CHARSET;
const PDO_DBNAME = PDO_DBNAME;
const PDO_DRIVER = PDO_DRIVER;
const PDO_HOSTNAME = PDO_HOSTNAME;
const PDO_PASSWORD = PDO_PASSWORD;
const PDO_USERNAME = PDO_USERNAME;

const CURRENT_TIMESTAMP = CURRENT_TIMESTAMP;
const PHP_AUTH_USER = PHP_AUTH_USER;

protected $id = null;
public $name = null;
public $email = null;
public $filename = null;
public $created_by = self::PHP_AUTH_USER;
public $modified_by = self::PHP_AUTH_USER;
public $date_created = null;
public $date_modified = null;
public $published = 1;
public $publish_up = null;
public $publish_down = null;
public $ordering = null;

/**
 * Getters for Protected Vars
**/
public function getId()
{
    return (int)$this->id;
}

/**
 * Save
 * @return boolean $query
 */
public function save()
{
    if(! (isset($_SESSION['errors']) AND !empty($_SESSION['errors']))){
        $db = new PDO(self::PDO_DRIVER . ':host=' . self::PDO_HOSTNAME . ';dbname=' . self::PDO_DBNAME, self::PDO_USERNAME, self::PDO_PASSWORD, array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES ' . self::POD_CHARSET));
        $values = array(
            ':name' => $this->id_parent,
            ':email' => $this->email,
            ':filename' => $this->filename,
            ':created_by' => self::PHP_AUTH_USER,
            ':modified_by' => $this->modified_by,
            ':date_created' => $this->date_created,
            ':date_modified' => self::CURRENT_TIMESTAMP,
            ':published' => $this->published,
            ':publish_up' => $this->publish_up,
            ':publish_down' => $this->publish_down,
            ':ordering' => $this->ordering,
            ':id' => $this->id
        );
        if((int)$this->id > 0){
            $sql = '
                UPDATE contacts
                SET
                    name = :name,
                    email = :email,
                    filename = :filename,
                    created_by = :created_by,
                    modified_by = :modified_by,
                    date_created = :date_created,
                    date_modified = :date_modified,
                    published = :published,
                    publish_up = :publish_up,
                    publish_down = :publish_down,
                    ordering = :ordering
                WHERE 1
                AND id = :id
                LIMIT 1
            ';
            $stmt = $db->prepare($sql);
            $query = $stmt->execute($values);
        }else{
            $sql = '
                INSERT INTO contacts
                SET
                    name = :name,
                    email = :email,
                    filename = :filename,
                    created_by = :created_by,
                    modified_by = :modified_by,
                    date_created = :date_created,
                    date_modified = :date_modified,
                    published = :published,
                    publish_up = :publish_up,
                    publish_down = :publish_down,
                    ordering = :ordering,
                    id = :id
            ';
            $stmt = $db->prepare($sql);
            $query = $stmt->execute($values);
            $this->id = $db->lastInsertId();
        }
        $db = null;
        return (bool)$query;
    }
    return false;
}

/**
 * Delete
 * @return boolean $query
 */
public function delete()
{
    if((int)$this->id > 0){
        $db = new PDO(self::PDO_DRIVER . ':host=' . self::PDO_HOSTNAME . ';dbname=' . self::PDO_DBNAME, self::PDO_USERNAME, self::PDO_PASSWORD, array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES ' . self::POD_CHARSET));
        $sql = '
            DELETE t, tt 
            FROM contacts AS t
            WHERE 1
            AND t.id = :id
        ';
        $stmt = $db->prepare($sql);
        $query = $stmt->execute(array(
            ':id' => $this->id
        ));
        $db = null;
        return (bool)$query;
    }
    return false;
}


/**
 * fetchRow
 * @return $this
 */
public function fetchRow($id = null)
{
    $db = new PDO(self::PDO_DRIVER . ':host=' . self::PDO_HOSTNAME . ';dbname=' . self::PDO_DBNAME, self::PDO_USERNAME, self::PDO_PASSWORD, array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES ' . self::POD_CHARSET));
    $sql = '
        SELECT * 
        FROM contacts
        WHERE 1
        AND id = :id 
        LIMIT 1
    ';
    $stmt = $db->prepare($sql);
    $stmt->execute(array(
        ':id' => (int)$id
    ));
    if($row = $stmt->fetch(PDO::FETCH_OBJ)){
        foreach($row as $i => $e){
            if($e != "") $this->{$i} = $e;
        }
    }
    $db = null;
    return $this;
}
}

Answer 1

The main security concern here I guess is protection from sql injection. Good news. You got that part right by using bound parameters.

Here is what you should improve and why:

Make the name of the class match what it is. In this case, it represents 1 Contact. So, name it Contact.

You said "besides no use of getters and setters". However, I'll point out that the reason you should use them is to provide a stable api to your calling code and to encapsulate future behavior. There are more reasons, but those are important ones.

Inject an instance of PDO instead of creating it all over the place. For one, all those connections will kill your performance. And, it will separate the concern of making the connection from this class - making it simpler.

public function __construct(PDO $pdo)
{
    $this->pdo = $pdo;
}

You will likely have other kinds of classes that represent different kinds of rows. Move the common behavior (find, save, delete) into a parent class.

Create proper doc blocks for your methods:

/**
 * @param int|string $id
 * @return null|Contract
 */
public function fetchRow($id)

Add proper error handling for PDO. You can configure PDO to throw exceptions. Or, if not, you should do checks like:

$stmt = $this->pdo->prepare(...);
if (! $stmt) {
    //handle this error situation
}

Remove direct access to $_SESSION. This kind of global access is very difficult to maintain and test in the long run.

Now, having said all this, your approach here (which is sort of an active record pattern) has been done many times over. Check out a library such as php active record and you will save yourself countless hours and hassle.