3
\$\begingroup\$

I have to write a Dog Creator class. I initially wrote this code:

<?php 
class Dog
{
  private $id, $name, $size, $age;
  public function setId($id){
    $this->id = $id
  }
  public function setName($name){
    $this->name = $name;
  }
  public function setSize($size){
    $this->size = $size;
  }
  public function setAge($age){
    $this->age = $age;
  }
  public function getId(){
    return $this->id;
  }
  public function getName(){
    return $this->name;
  }
  public function getSize(){
    return $this->size;
  }
  public function getAge(){
    return $this->age;
  }
}

class DogMapper
{
  private $pdo;

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

  public function insert(Dog $dog){
    if(/* my controls*/){
      $this->pdo->query(/*query to insert $dog to database*/);
      $dog->setId($this->pdo->lastInsertId);
      return true;
    }else{
      return false;
    }
  }

  // Other CRUD methods


}

?>

Then I wrote one class instead of two:

<?php 
class Dog
{
  private $id, $name, $size, $age, $pdo;

  public function __construct(PDO $pdo){
    $this->pdo = $pdo;
  }
  public function setName($name){
    $this->name = $name;
  }
  public function setSize($size){
    $this->size = $size;
  }
  public function setAge($age){
    $this->age = $age;
  }
  public function getId(){
    return $this->id;
  }
  public function getName(){
    return $this->name;
  }
  public function getSize(){
    return $this->size;
  }
  public function getAge(){
    return $this->age;
  }
  public function insert(Dog $dog){
    if(/* my controls*/){
      $this->pdo->query(/*query to insert $dog to database*/);
      $this->id = $this->pdo->lastInsertId;
      return true;
    }else{
      return false;
    }
  }
  // Other CRUD methods
}
?>

What is the better version? And what improvements can I make? Furthermore, some people have told me that I should not always use the getter and setter but the constructor, in my case, should I use them? If I should not use them, why and how do I know when I have to use them and when not?

\$\endgroup\$
1
\$\begingroup\$

The first approach seems to better separate concerns. You have one class representing a dog and another class which can perform database operations related to the dog model.

I don't know that DogMapper is a meaningful name though. I don;t see where any sort of mapping operation is happening. Perhaps DogFactory, DogProvider, DogModel or similar might be more meaningful.

In either case I have a challenge with the level of mutability given to the Dog object. Are these objects supposed to be fully mutable by calling code and then the are only written to the database when calling code specifies (like for initial insert or later for update)? I see nothing that prevents the Dog from being reinserted into the database over and over again. So, should you throw an exception of Dog with id already set is passed to insert?

\$\endgroup\$
1
\$\begingroup\$

Naming

I think "DogMapper" is quite good with the name. What your are doing is you are transforming a relational representation of a Dog to an object in the sense of php. In Java this is called Object-Relational-Mapping.

Getter and setter

Getter and setter introduce an indirection that allow you to perform additional actions. But doing this the things are getting complicated. Mostly you will violate the Single Responsibility Principle because your getters and setters should only do what they are telling. It depends on the type of object.

In Java we call "objects" that have setters and getters for all their attribute "Beans". They have little assertions to consistency.

As setters may put your Beans into an inconsistent state you should think about it if you want to allow this. If you do not want to allow this your Dog-Class seems to be a "Business-Object" or a "Domain-Object" that holds its internal state consistent. Setters in Business-Object will evaluate the input. And Getters of Business-Objects may do more than simply returning a the value of an attribute. They can do calculations or load data.

Mapping

You DogMapper-class seems to me like a DAO. A DAO will access the underlying datastore and pass an object with a proper representation to the upper layer.

Semantic

The question is: Should your Dog-Class be a "Bean-Object" or a "Domain-Object"?

My personal opinion is: Separate the concerns of the Object-Relational-Mapping and the business logic and do not put all the burden on one object.

So you have three responsibilities:

  1. A Business-Object "Dog" that is self organized to keep its internal state consistent
  2. A Mapping-Object "DogMapping" that represents an intermediate state with little assertions to consistency. Its purpose is to make it easy to persist, load and transform it to a Business-Object
  3. A DAO-Object "DogDAO" (former name: DogMapper) that abstracts from the data layer. It can be used from the business layer.
\$\endgroup\$
1

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.