10

Say I have a very simple CRUD system in PHP to manage a database. Say it holds a list of products. Using Doctrine ORM, I'd like to query the database and view/edit/add records. According to the Getting Started manual,

When creating entity classes, all of the fields should be protected or private (not public), with getter and setter methods for each one (except $id). The use of mutators allows Doctrine to hook into calls which manipulate the entities in ways that it could not if you just directly set the values with entity#field = foo;

This is the sample provided:

// src/Product.php
class Product
{
    /**
     * @var int
     */
    protected $id;
    /**
     * @var string
     */
    protected $name;

    public function getId()
    {
        return $this->id;
    }

    public function getName()
    {
        return $this->name;
    }

    public function setName($name)
    {
        $this->name = $name;
    }
}

// Recording a new title
$product->setName("My new name");
$db->persist($product);
$db->flush();

// Echoing the title
echo $product->getName();

However, this seems overly complicated. Say I don't have a need to hook into calls to manipulate entities, as explained in the manual. I can make this code much shorter and do this:

// src/Product.php
class Product
{
    /**
     * @var int
     */
    public $id;
    /**
     * @var string
     */
    public $name;
}

This would allow things like this:

$product = $db->getRepository('Product')->find(1);

// Recording a new name
$product->name = "My new title";
$db->persist($product);
$db->flush();

// Echoing the title
echo $product->name;

The advantages are:

  • Always using the exact same name
  • No extra setters and getters in the entity class

What is the disadvantage of using this? Am I taking certain risks here?

Pieter van den Ham
  • 4,381
  • 3
  • 26
  • 41
user32421
  • 689
  • 3
  • 8
  • 19
  • 1
    Nothing wrong with using public variables in spite of the posts on encapsulation. However, doing so means lazy loading no longer works as Doctrine relies on extending getters in order to implement lazy loading. As long as you always load your data in advance then this is not a problem. In fact, lazy loading is often something to avoid. – Cerad Sep 03 '17 at 13:19
  • Thanks but there are various answers that imply otherwise. So I'm quite confused now as to what is the right answer...?! – user32421 Sep 04 '17 at 08:25
  • The other answers are mainstream. You can go with them by default. Once you really understand the arguments then you can chose your own way. In programming, there is seldom a single "best" way. – Cerad Sep 04 '17 at 13:34

2 Answers2

7

Say I don't have a need to hook into calls to manipulate entities, as explained in the manual.

You do not need to hook into these calls, but Doctrine does. Doctrine internally overrides these getters and setters to implement an ORM as transparently as possible.

Yes, it is overly complicated, but that is the PHP language at fault and not Doctrine. There is nothing inherently wrong with using public properties, it is just that the PHP language does not allow creating custom getters/setters the way some other languages do (such as Kotlin, C#).

What does Doctrine do exactly? It generates so-called Proxy objects to implement lazy loading. These objects extend your class and override certain methods.

For your code:

// src/Product.php
class Product
{
    /**
     * @var int
     */
    protected $id;
    /**
     * @var string
     */
    protected $name;

    public function getId()
    {
        return $this->id;
    }

    public function getName()
    {
        return $this->name;
    }

    public function setName($name)
    {
        $this->name = $name;
    }
}

Doctrine might generate:

class Product extends \YourNamespace\Product implements \Doctrine\ORM\Proxy\Proxy
{

    // ... Lots of generated methods ...

    public function getId(): int
    {
        if ($this->__isInitialized__ === false) {
            return (int)  parent::getId();
        }


        $this->__initializer__ && $this->__initializer__->__invoke($this, 'getId', []);

        return parent::getId();
    }

    public function getName(): string
    {

        $this->__initializer__ && $this->__initializer__->__invoke($this, 'getName', []);

        return parent::getName();
    }

    // ... More generated methods ...
}

(What this does exactly is not the point, the point is that Doctrine MUST be able to override getters and setters to implement an ORM)

To see for yourself, inspect the generated Proxy objects in the proxy directory of Doctrine. See the Advanced Configuration section for more info on configuring the proxy directory. Also see Working with Objects for more info on Proxy objects and how Doctrine utilizes them.

Pieter van den Ham
  • 4,381
  • 3
  • 26
  • 41
  • Thanks but in one of the comments it was said that the main reason Doctrine needs this is for lazy loading, and that if I don't need that, I can make them public. Is that the same you are saying? I must say I have create a few sites with public variables and it all seemed to work very well. I was surprised to find out I had to make them private... – user32421 Sep 04 '17 at 08:29
  • @user32421 Yes, one of the reasons Doctrine does this is for lazy loading. But *also* for tracking whether an object is "dirty" (e.g. it has changed properties) by overriding the setter. You simply cannot work around this. – Pieter van den Ham Sep 04 '17 at 08:47
  • Note that the documentation says that you *should* make them protected/private. Nothing withholds you from making them public BUT Doctrine would have no way of knowing that you changed the properties of your object and thus would have no way of knowing *which* objects it should update in the database and/or which properties it should load from the database. – Pieter van den Ham Sep 04 '17 at 08:49
  • The whole point of these getters/setters is that they allow Doctrine to track the objects state, because there is no other way to do this in the PHP world. – Pieter van den Ham Sep 04 '17 at 08:50
  • 1
    @Pete - Your comment about how Doctrine tracks "dirty" objects is wrong. http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/change-tracking-policies.html Lazy loading is the only thing you lose. – Cerad Sep 04 '17 at 13:37
  • @Cerad Ah thanks! Didn't know. Well it is kinda obvious now that I think about it :) – Pieter van den Ham Sep 04 '17 at 13:43
1

Why not use public entity classes in Doctrine?

Actually what you are asking is:

Where does this getters/setters come from?

It comes from the Encapsulation in OOP.

so, what is encapsulation?

It’s the way we define the visibility of our properties and methods. When you’re creating classes, you have to ask yourself what properties and methods can be accessed outside of the class.

Let’s say we had a property named id. If a class extends your class, is it allowed to manipulate and access id directly? What if someone creates an instances of your class? Are they allowed to manipulate and access id?

NO! They should not be able to reach the id attribute directly.

Why not?

Example: What happens if you only accept Invoice Ids which start with "IX" and other classes have access to your class and they set the invoice_id directly?

What happens if they accidentally set the invoice_id to "XX12345"?

of course, you cannot do anything (like validating the input), because you are using public properties and no setters.

but if you had a setter method in your class, you could just do

private $invoiceId;

public function setInvoiceId($invoiceId) 
{
    if (is_null($invoiceId) || preg_match("#^IX(.*)$#i", $invoiceId) === 0)
        return false;

    $this->invoiceId = $invoiceId;

    return $this;
}

I hope it was clear enought. I will try to extend the answer

BenMorel
  • 34,448
  • 50
  • 182
  • 322
Vural
  • 8,666
  • 11
  • 40
  • 57
  • Thanks, but I'm still not 100% clear on this. You mention that others should not be able to manipulate the attributes. However, what others? I'm the only developer, this is a standalone website with no third-party access....? And I can see the advantage of being able to apply these extra operations on invoice IDs, but can't I just change the code in the (in my case very unlikely) case that I actually need it? – user32421 Sep 04 '17 at 08:28
  • If you even a standalone developer, will you remember everything later if you don't keep the standards? like OOP-Standards, PSR, Principles.. etc.. Maybe one day other developers will join you or you will need an API that needs to use your Classes/Entities.. etc. – Vural Sep 04 '17 at 13:35
  • 2
    "It comes from the Encaptulation in OOP." No. It comes from the fact that Doctrine can't make proxy for public property. So it's just technical issue. Domain model with setters still violates encapsulation in that sense that we don't put behavior (methods that manipulates the data) into aggregate root. We manipulate data from outside by calling low-level setters/getters. Point of encapsulation to combine both in the single unit. See https://www.doctrine-project.org/projects/doctrine-orm/en/latest/tutorials/getting-started.html#adding-behavior-to-entities – Misanthrope Aug 07 '18 at 19:19