2
\$\begingroup\$

Once a project gets bigger than a couple pages you don't want to have to call up every single page and do find & replace every time there's a change in the database structure. At least, I don't!

My solution has been to make a db handle class series that contains names as variables and queries as methods. But, I'm not very good with classes. I think I'm not coming at it from the right direction.

Sample:

class Users {
    //table
    protected $users = 'users';
        //columns
        protected $user_id = 'user_id';
        protected $username = 'user_name';
        protected $password = 'user_password';
}

class Emails extends Users {
    protected $emails = 'emails';
        protected $address = 'email_address';
        protected $confirm = 'email_confirmed';
        //the foreign key user_id is already defined in users

    function getUserEmail($username) {
        $join = junct([$this->users, $this->emails], $this->user_id);
        $sql = select($join, [$this->address], [$this->username]);
        $result = prepped_query($sql, $username);
        return $result;
    }
}

$x = new Emails();
$email = $x->getUserEmail($username);

One problem so far is that by having a class per table, I can use inheritance for one-to-many tables but many-to-many tables I can't inherit both. It makes awkward joins and forces prioritization where there sometimes isn't.

I have a couple ideas how I could restructure (defining all tables and names in one parent class with methods in children is one idea) but I don't know that any of them would actually be better, and they could have their own problems. Suggestions appreciated.

\$\endgroup\$
  • \$\begingroup\$ It might help if you show your schema. \$\endgroup\$ – Mike Brant Mar 26 '17 at 15:46
  • \$\begingroup\$ The schema is in the class. It's how I'm building them. I could add one showing a many to many relationship, like users and events, if you think it'd be helpful. \$\endgroup\$ – citrusy Mar 26 '17 at 21:13
  • \$\begingroup\$ Not related with the code review, but do you know ORM as Doctrine or Propel? This looks to me like reinventing the wheel. \$\endgroup\$ – Chococroc Mar 26 '17 at 22:56
  • \$\begingroup\$ Honestly never heard of them before. And it's not like I didn't search? I've literally spent days searching before, but the results are always a different kind of database class. Propel does look similar. :/ \$\endgroup\$ – citrusy Mar 27 '17 at 2:27
  • \$\begingroup\$ I fail to see how this code is to prevent find & replace. And yes, what you're looking for is called ORM and there are plenty of them. \$\endgroup\$ – Your Common Sense Mar 27 '17 at 7:58
0
\$\begingroup\$

I am challenged by a few things here:

  • First the inheritance model makes no sense in real world terms. Why would an email class inherit from a user class? Try to think in real-world terms about the objects in your system and you will more often than not, end up with a better design. Here I would think that a user class could have a property that holds the email object(s) that belong to the user, but I don't see how an email, when defined in terms of it's own object would inherit from a user, which would have a wholly separate set of properties it has and actions (methods) it can perform than what an email object does. So why use inheritance here at all?
  • You don't really consider what might truly be common amongst two wholly different objects that exist in your database, that might be points for common inheritance. What do users and emails have in common?
    • They both represent a database table
    • They both have columns
    • They both have some unique index field
    • They likely both need methods to populate (saturate) the object based on some lookup criteria (by unique id for example).

So what does this mean? It means you probably want to consider having a base class (typically abstract ) that is usually what is called a "model" (the "M" part of "ORM" as mentioned in the comments to the question above). A typical model class might present some skeleton functionality like:

  • table property (or class constant) that defines the table in DB that the class represents
  • primary_key property (or class constant) the defines the primary key field for the table
  • various class properties (or an array/object of properties) that represent the field available on the object
  • a findById() method that does basic primary key lookup and object saturarion that might have a common implementation amongst inheriting models.
  • a delete() method
  • other methods that might represent common object behavior.

A very simple model skeleton might look like

abstract class Model
{
    // DB schema fields
    protected static $table = null;
    protected static $primaryKey = null;
    // could also have other static field like
    // properties to express relationships (foreign key/tables)
    protected static $hasMany = null;
    protected static $belongsTo = null;

    public function __construct($primaryKey = null) {
        // optional allow instantiation by passing primary key
        if (!is_null($primaryKey)) {
            return self::getByPrimaryKey($primaryKey);
        }
    }

    public static function getByPrimaryKey($primaryKey) {
        self::validatePrimaryKey($primaryKey);
        $fields = self::getObjectFields(true);
        // here is simple case
        // perhaps this code varies if there are relationships and you want to
        // populate related objects as well here.
        $sql = '
SELECT `' . self::$primaryKey .'` AS id, `' . implode('`, `', $fields) . '` 
FROM `' . self::$table . '`
WHERE `' . self::$primaryKey . '` = :id
LIMIT 1';

        // not shown - execute prepared statement against DB using $sql
        // assume data is populated into $row
        // saturate and return object if successful
        $returnObj = new self();
        foreach($row as $field => $value) {
            $returnObj->{$field} = $value;
        }
        return $returnObj;
    }

    // override if you need to "hide" certain fields from being saturated
    // requires PHP >= 5.3.0 due to reliance on late static binding  
    public static function getObjectFields() {
        $properties = get_object_vars(get_called_class());
        // remove id field, which we will consider as standard field name for
        // primary key and thus needing special treatment
        unset($properties['id']);
        return array_keys($properties);
    }

    // override this function on per-class basis if primary key is not 
    // integer value
    public static function validatePrimaryKey($primaryKey) {
        if (!is_int($primaryKey)) {
            throw new InvalidArgmuentException(
                'Integer primary key value required.'
            );
        }    
    }

    // maybe some other function you want to force inheriting class to implement
    public static abstract function create() {}
    public static abstract function delete($primaryKey) {}
    public abstract function update() {}
    // etc.
}

class User extends Model
{
    protected static $table = 'users';
    protected static $primaryKey = 'user_id';
    protected static $hasMany = [
        // field name => class info
        'emails' => [
            'class' => 'Email',
            'key' => ['email_id']
         ]
    ];

    protected $id = null;
    protected $name = null;
    protected $passwordHash = null;
    protected $emails = [];

    // perhaps override here to provide logic to also saturate emails
    // if you did not implement that generically in Model
    public static function getByPrimaryKey($primaryKey) {
        // some logic
    }

    // implement abstract functions from base class
    public static function create() {
        // some logic to create a new DB record of this type
    }

    public static function delete($primaryKey) {
        // some logic to delete object of given primary key from DB
    }

    public function update() {
        // some logic to persist object state to DB
    }

    // perhaps some custom methods
    public static function getUserByEmail() {
        // would require working with relate email table a condition for
        // returning User object
    }

    // perhaps some logic to retrieve use object based on login credential
    // or return null if invalid
    public function getUserByLogin($username, $password) {
       // some logic
    }

    // and so on...

    // some getters
    public getName() {
        return $this->name;
    }

    public getPassswordHash() {
        return $this->passwordHash;
    }

    public getEmails() {
        return $this->emails;
    } 

    // and so on...
}

class Email extends Model
{
    protected static $table = 'emails';
    protected static $primaryKey = 'email_id';
    protected static $belongsTo = [
        'User'
    ];

    protected $address = null;
    protected $confirmed = null;

    // not shown - implement abstract methods
    // not shown - getters
}

Again, this is pretty basic representation of a typical model inheritance approach. You can look into popular PHP ORM's to get more full-featured behavior and to get a better idea for how they handle table realtionships and such.


By the fact that you are using password field instead of passwordHash or similar, I am wondering if you are not properly hashing your passwords. This should be considered mandatory for storage of password data.

\$\endgroup\$
  • \$\begingroup\$ Emails are only used to confirm user/reset passwords and must be unique to each user, a user can have more than one email though. Regarding passwords I do hash them, it's just not named that way. \$\endgroup\$ – citrusy Mar 27 '17 at 17:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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