1

If we have a code like this:

class Game {
    private $_id;
    private $_name;
    private $_url;

    public function __construct($_id,$_name,$_url){
        $this->_id = $_id;
        $this->_name = $_name;
        $this->_url = $_url;
    }
}

And we want to simply connect to our Database to get a game by id, where do we place the 'getByID' function?

Do we place it within the 'Game Class' as 'static function', do we put it in the 'Database Connection Class' as 'public function' or do we just put the method in the 'general functions inside the main index.php' as 'function'?

I currenctly have choosen for a 'static function' within the 'Game Class':

public static function getByID($id,$db){    
    $query = "SELECT * FROM game WHERE id = :id LIMIT 1";
    $prepare = array(":id"=>$id);
    $result = $db->Precute($query,$prepare);
    foreach($result as $r) return new Game($r['id'],$r['name'],$r['url']);
    return null;
}

(Precute is a custom function within the Database Class to prepare and execute the query) How would you approach this?

9
  • Remove the quote in $this->_url = $_url'; for one thing. That will surely throw an error, unless that's a typo. Commented Jan 5, 2014 at 16:31
  • I think a static method is fine . P.S - did you consider using a php framework ? It's exactly these kind of code organization problems that a framework will solve for you... Commented Jan 5, 2014 at 16:33
  • Thank you Fred -ii-, I've corrected the typo. Commented Jan 5, 2014 at 16:36
  • You're welcome. @MaartenSchermer That's why I always mention "unless that's a typo" ;-) Commented Jan 5, 2014 at 16:38
  • 2
    @NielsKeurentjes , how comes that you picked the worst possible answer? Commented Jan 5, 2014 at 21:08

4 Answers 4

2

In proper OOP, a DAL function which returns an instance of a specific class should be static within that class. As a base rule, all functionality related to one specific object should be part of that specific object, as an instance method if invoked on instances or a static method if it creates or manages instances ('factory pattern').

Your function isn't static currently, correct usage would be:

class Game
{
   ..other functions..

   public static function getById($id)
   {
      ..implementation, which can either access central storage or retrieve
        the object itself if concurrent edits are not an issue..
   }
}

Then elsewhere:

$myGame = Game::getById(684);

You may want to have a look at Doctrine instead of re-inventing the wheel. And even if you do want to make a new wheel, its code samples all follow correct OOP principles.

Sign up to request clarification or add additional context in comments.

28 Comments

No, that's not the way to do it.
@Anyone There is no such thing as Single Responsibility Pattern ;)
I don't need to click links to know what it is ;)
Thank you mr. Keurentjes for your fast answer and your tips ;-)! You have answered my question and therefore I will accept it. Have a nice day!
-1: recommendation of pointless framework and examples with code, that depends on global state.
|
1

This Answer takes another approach. Instead of getting Objects from Static Factory. This solution takes a approach of creating a blank object and then calling the database methods to make the object a live representation of a actual row.

first the observations from your question -

an Object/Instance of Game class represents a Row of Table game. And the Game class itself can be taken as a representation of `game' table.

If the above observation is correct along with the assumption that there are more tables with a representation in class hierarchy. You should have a class to represent generic 'Table'

class Table {   //The class itself can be made abstract depending upon the exact implementation 
   protected $_tableName;
   protected $_connectionParams;
   protected $idAttribute = 'id';

   public function __construct($tableName, $connectionParams, $idAttribute){
       $this->_connectionParams = $connectionParams;
       $this->_tableName = $tableName;
       if(isset($idAttribute)) {
          $this->idAttribute = $idAttribute;
       }
   };

   private function _getConnection() {
      //return $db using $_connectionParams
   };

   public function getByID($id) {    
      $this->getByKeyVal($this->idAttribute, $id);
   };

   public function getByKeyVal($key, $val) {
      $query = "SELECT * FROM ". $this->_tableName ." WHERE `". $key ."` = :key LIMIT 1";
      $prepare = array(":key"=> $val);
      $result = $this->_getConnection()->Precute($query,$prepare);
      $this->processRow($result[0]);
   };

   //This needs to be overridden
   public function processRow($row) {
      return true;
   };
}

Now extend the generic Table class for Game Table

class Game extends Table {
   private $_id;
   private $_name;
   private $_url;
   public function __construct($defaults) {
      if(isset($defaults) {
         if(is_array($defaults)) {
            $this->processRow($defaults);
         } else {  
            $this->getByID($defaults);
         }
      } else {
         //Some default setup here if requried
      }
      $connectionParams = [];  //Prepare Connection Params here
      parent::__construct('game', $connectionParams);
   };

   //Override processRow
   public function processRow($row) {    
      if(isset($row['id']) {
         $this->_id = $row['id'];
      }
      $this->_name = $row['name'];
      $this->_url = $row['url'];
   };
}

Above is a very rough example. The actual Class structure will depend upon your requirements. But the general rule of thumb is to treat a Class as a blueprint of a concrete object. And all the methods related with a Generic Classification should go in there own class.

The getConnection Method itself can be put into a seprate DB connection class and inserted in table via a either mixin pattern or generic class inheritance.

Use the above setup like this

$game_new = new Game();  // for blank object  --- for a new row

$game_435 = new Game(435);    //row with 435 ID

$game_default = new Game(array(    //new row with defaults
  'name' => 'Some Name',
  'url' => 'Some Url'
));

7 Comments

Please comment alongside down-vote if something is wrong with this approach.
This is a rather nice approach. I've used this before together with ActiveRecord. The only major change I'd do is replace getById($id) with a getBy($key, $val) one, since [1] sometimes you might not have an id as the PK and [2] sometimes you would want to load the item by a different criteria. Oh, and instead of Table, I'd make it more abstract so one can load from different places, not just DB.
Thats correct, i normally keep a protected $_idAttribute. i'll update the answer.
Yes. I tried to keep it as much close to OP's approach. The most ideal way is to define a DataReader interface. have different DataReader for database, cache, some external API, and use the DataReader as mixin. the DataReader switch can be made during the lifetime of object.
I think the 'getByKeyVal($key, $val)' function would be very usefull indeed! Although I think this is a pretty difficult approach vs the static function @Niels Keurentjes provided.
|
0

What you want is a "bucket" full of Game objects. When ever you want a Game Object (representing data in your database), you ask your "bucket" to give it to you. Let me give you an example of how Doctrine2 implements this: http://docs.doctrine-project.org/en/2.0.x/reference/working-with-objects.html

So where you want to place your "getById" (or as I would do "findById"), is in your "bucket".

// lets presume that the em is an instance of \Doctrine\ORM\EntityManager
// The entity manager does what the name says.
$id = 1234;
$game = $entity_manager->find('MyNamespace\Entity\Game', $id);
$game->setName('My first game!');
// We now tell the em to prepare the object for pushing it back to the "bucket" or database
$entity_manager->persist($game);
// Now we tell the em to actually save stuff
$entity_manager->flush();

This should give you an indication of how to use it. Objects follow the Single Responsibility Principle. You don't ask an object to retrieve itself. You ask the "bucket" to retrieve you an Object.

http://en.wikipedia.org/wiki/Single_responsibility_principle

7 Comments

This answer is actually completely offtopic. Nice how you explain that an entity manager can eliminate overhead et al, but in the end - the call to $entity_manager->find(...) should be in Game::getById. Which is the question, not how to build an entity manager like Doctrine has.
No, because telling OP how to make a static function INSIDE the object it wrong. It solves his problem, but doesn't teach him the proper way. How do you plan on mocking your entities now? Why does your entity know about the database? How would you even serialize this? You can't serialize a connection.
The OP's question is, literally, 'where do we place the 'getByID' function'. You're answering an awful lot of questions, except the only one the OP asked.
"where do we place the 'getByID' function?" You place it in the "bucket" aka entity manager. NOT in the entity itself.
@NielsKeurentjes actually "the worst" would be using some sort of static calls. LIKE YOU DID IN YOUR OWN ANSWER!
|
0

What if I told you that there are more beautiful ways to put things on their places. A very simple case might contain 3 basic components to work:

  1. Db framework - Which handles data access.
  2. Table repsotor classes - Which know how to map classes to tables, how to create classes from table data and how to create data from table classes.
  3. Model or business layer which contain actual classes.

For better understanding imagine you have database object mapper framework. The framework can be far complex but in few lines we can demonstrate how it`s basic concepts work.

So the 'Framework':

<?php

//This class is for making link for db framework
class link
{
    public $link;
    public function __construct ($hostname, $database, $gamename, $password)
    {
        $this->link = new \PDO ('mysql:host='.$hostname.';dbname='.$database, $gamename, $password);
        $this->link->query('use '.$database);
    }
    public function fetch ($query)
    {
        $result = $this->link->query($query)->fetch();
    }
    public function query ($query)
    {
        return $this->link->query($query);
    }
    public function error ()
    {
        return $this->link->errorInfo();
    }
}

//This class collects table repositories and connections
class database
{
    public $link;
    public $tables = array ();
    public function __construct ($link)
    {
        $this->link = $link;
        table::$database = $this;
    }
}

//This is basic table repositor class
class table
{
    public static $database;
}

?>

Now as we have our db framework let us make some table repositor which knows how to save/load/delete game:

class games extends table
{
    public function create ($row)
    {
        $return = new game ();
        $return->id = $row[0];
        $return->name = $row[1];
        var_export($row);
        return $return;
    }
    public function load ($id=null)
    {
        if ($id==null)
        {
            $result = self::$database->link->fetch("select * from games");
            if ($result)
            {
                $return = array();
                foreach ($result as $row)
                {
                    $return[$row[0]] = $this->create($row);
                }
                return $return;
            }
        }
        else
        {
            $result = self::$database->link->fetch("select * from games where id='".$id."'");
            if ($result)
            {
                return $this->create(reset($result));
            }
            else
            {
                echo ("no result");
            }
        }
    }
    public function save ($game)
    {
        if (is_array($save))
        {
            foreach ($save as $item) $this->save ($item);
        }
        if ($game->id==null)
        {
            return self::$database->link->query("insert into games set
                                                 name='".$game->name."'");
        }
        else
        {
            return self::$database->link->query("update games set name='".$game->name."'
                                                 where id='".$game->id."'");
        }
    }
    public function delete ($game)
    {
        self::$database->link->query ("delete from games where id='".$game->id."'");
    }
}

Now we can make our model which in this case will contain actuall game class.

class game
{
    public $id;
    public $name;
    public function __construct ($name=null)
    {
        $this->name = $name;
    }
}

And than actually use it:

$database = new database (new link('127.0.0.1', 'system_db', 'root', '1234'));
$database->tables['games'] = new games();

if (!$database->tables['games']->save (new game('Admin')))
{
    var_export($database->link->error());
}

var_export($database->tables['games']->load(2));

For the moment I prefere this pattern for working with db in my projects. Using it I can achieve that my actuall business objects(In this case class game) will know nothing about where and how they are saved. This gives me an ability to be indipendent from actuall storage and focus on project logics.

Also there is one lightweight framework so called db.php (http://dbphp.net) and it even gives me ability to avoid to write table repositories and even creates/modifies tables needed for my business classes on the fly but uses almost same concept I described here.

1 Comment

Nice one! Why not use prepared statements?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.