Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

I have a design question for working with databases in object-oriented PHP.

I have a table in my database called products. Then I have 2 classes for this table in my PHP project. ProductsRepository.php and Product.php.

ProductsRepository.php currently looks like this

class ProductsRepository
{

    private $db;

    public function __construct(DB $db)
    {
        $this->db = $db;
    }

    public function getById($id)
    {
        $data = $this->db->GetProductByID($id);

        $product = new Product(
            $data['id'],
            $data['category'],
            $data['title'],
            $data['description'],
        );

        return $product;
    }

    public function getAll()
    {
        $products = array();
        $data = $this->db->GetAllProducts();
        foreach ($data as $productData){
            $product = new Product(
                $productData['id'],
                $productData['category'],
                $productData['title'],
                $productData['description'],
            );
            $products[] = $product;
        }

        return $products;
    }
}

My question is how I should build a "createProduct" method. First and foremost, which class should it be in? I want the method to return an instance of the new product. The main problem I have with this is how I should handle the id of the new product since it will be given the next id available by the database.

A few options I've thought of:

Have createProduct in ProductsRepositoryand let it take a product with id set to -1 as argument

public function createProduct(Product $product)
{
    $product['id'] = $this->db->insertANewProduct($product); //returns the id
}

Have createProduct in Product

Should this be the constructor for the Product class?

public function createProduct($db, $data)
{
    $this->$data = $data; //includes all info of the new product except the id
    $this->$data['id'] = $db->insertANewProduct($data); //returns the id
}
share|improve this question

2 Answers 2

up vote 2 down vote accepted
$this->$data['id'] = $db->insertANewProduct($data);

I think is a bad practice, because breaks de Single Responsibility Principle, the product class knows it's been persisted. You are loosing what you win with the repository pattern. If you want to do that another option (less OO) is Active Record.

I would choose the option 1

    public function createProduct(Product $product){
       $product['id'] = $this->db->insertANewProduct($product);
       return $product;
    }

I really work in Java and use JPA for persistence my entities, in this case that you have auto generated id the JPA implementation (i.e. Hibernate) will populate the object id for you. Have you considered use a ORM?

share|improve this answer
    
Yes, I'm using an SQL database with AUTO_INCREMENT on the IDs –  Oskar Jul 16 at 23:21
    
And I imagine that you want to return the created product at success with the id? –  gabrielgiussi Jul 16 at 23:29
    
That is correct –  Oskar Jul 17 at 0:27
    
An ORM can solve many issues very well. But sometimes using an ORM is shooting birds with canons :) –  AnotherGuy Jul 17 at 11:56

I have encountered this issue several times and found the following solution at best. Please keep in mind this is my approach and your preference may be different.

I always try to keep any form of SQL interaction inside the repository. Therefore the create method should be declared inside the repository. Now the issue with the ID persists.

When looking at your code you are creating a Products instance a different places. This could be extracted into a factory method or even a separate factory class if required. The class approach would give you more wriggle room in the future, but its more verbose.

Using a factory would change implementation details without the consumer classes of your factory knows. Imagine you in the future would like the category to inside its own class. You would only have to change to factory to make this change. As long as the factory returns a valid instance of the specified class you can go crazy inside the factory.

The use of the factory can easily be implemented into your current methods. When fetching one or more products you already have the required parameters, so all it takes is to change a line.

I would then structure the create method like the following:

public function create($category, $title, $desc) {

    /*
     * Create the appropriate SQL query. 
     */
    $stmt = $this->pdo->prepare('...');

    // Bind parameters...

    /*
     * Use your preferred exception type.
     */
    if(!$stmt->execute()) {
        throw new \RuntimeException('Could not insert product into data storage!');
    }

    /*
     * Then fetch the ID from the database. Using PDO makes this very easy.
     * 
     * http://php.net/manual/en/pdo.lastinsertid.php
     */
    $id = $this->pdo->lastInsertId();

    /*
     * Return the new (valid) product from the factory.
     */
    return $this->factory->build($id, $category, $title, $desc);
}

There are some changes when using this method from your current design. The most obvious are the addition of a factory class. This should be injected during instantiation inside the __construct() method. The more subtle is an alternate database interaction.

Right now your database is becoming a god-class. From what I can see it knows how to fetch a single or more products. To keep class responsibility consistent the database class should only know how to serve a database connection (consider it an improved handler). Therefore have I used PDO as my alternative. This also makes it easier to perform specific tasks such as transactions without changing database code or creating new methods inside it.

I hope this approach can help you. Otherwise feel free to ask questions.

Happy coding!

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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