16

I remember reading that in Doctrine 2 models, I should not set properties/fields public. How then would you expose these fields? The sandbox used get*() & set*() methods. Is that the best idea? Its very cumbersome. Using magic methods __get() __set() will make things similar to setting fields public?

Whats your recommendation?

Jiew Meng
  • 84,767
  • 185
  • 495
  • 805
  • There are many other good reasons why you shouldn't make your class properties public as well! Most obvious is that if they're public I can set their values to be absolutely anything I want with no regard for what they should be. For example if property $foo is supposed to be an object of class Bar but is public then how do you prevent someone else from setting it to be anything other than an instance of class Bar? I could make it an instance of class Quux, an integer, a function... As for __get and __set, magic makes code a lot more difficult to understand and should be avoided. – GordonM Jul 01 '14 at 11:29

6 Answers6

13

Here's why you can't use public properties: How can public fields “break lazy loading” in Doctrine 2?

You are correct that __get() and __set() can make accessing the protected/private fields easier.

Here's a simple example:

public function __get($name)
{
  if(property_exists($this, $name)){
    return $this->$name;
  }
}

Of course that gives access to all the properties. You could put that in a class that all your entities extended, then define non-assessable fields as private. Or you could use an array to determine which properties should be accessible:$this->accessable = array('name', 'age')

There are plenty of ways to keep all properties protected and still have a reasonably easy way to get/set them.

Community
  • 1
  • 1
Tim Lytle
  • 17,549
  • 10
  • 60
  • 91
  • 9
    I would advise against using __get and __set in your entities just because you are too lazy to write them yourself. There are instances where it might make sense to use magic method, but that depends on the entities purpose. Most IDEs can generate them for you. – Cobby Feb 10 '11 at 07:17
  • 6
    @Cobby Official doctrine2 documentation states: `...We therefore urge you to map only private and protected properties on entities and use getter methods or magic __get() to access them.` http://www.doctrine-project.org/docs/orm/2.0/en/reference/best-practices.html – Petr Peller May 20 '11 at 12:08
  • 3
    @Cobby Have you ever heard about inheritance? I still can't see a reason why is it better (automatically or not) write hundreds lines of code than write about 20 in a BaseEntity class. – Petr Peller May 22 '11 at 11:55
  • 2
    You don't have to write code, IDEs generate it. Otherwise it takes about 3 minutes to type it up yourself. – Cobby May 22 '11 at 23:53
  • @Tim Lytle, instead of using $this->accessable we can use current ClassMetadata::$fieldNames with the list of our object fields – Mikl May 30 '14 at 18:57
  • Just a thought... You could technically use the magic getter and setter to call something like `$function = 'get' . ucfirst($name); $this->$function();` I know this doesn't help you NOT write code... but there's no reason you couldn't make a check to see if the function didn't exist and then pull in the property instead. This will make it so you can overload those getters and setters without making a direct mapping where you need it and maintain the beautifully short `$object->property` syntax. – General Redneck Feb 12 '18 at 02:37
9

Personally, I don't like boilerplate code with trivial purpose - it makes the code ugly and tiresome to read. Therefore, I strongly prefer __get/__set. That said, they do have a few drawbacks:

  • they are significantly slower than normal function calls, though not so much that it should make a difference in practice, as database access is several orders of magnitude slower
  • __get/__set only gets called when the field is not visible; if you access properties in the code of the entity class, they do not get called, and the proxy has no chance to load itself. (Doctrine tries to avoid this by instantly loading the proxy as soon as one of its public methods are called, but there are some exceptions such as __construct or __wake where that would not make sense, so you can get into trouble by e.g. reading a field in the constructor.)
  • PHP has some confusing behaviors related to magic methods - e. g. empty($entity->field) will not invoke __get (and thus it will break proxy behavior if used)
Tgr
  • 27,442
  • 12
  • 81
  • 118
  • 2
    BTW: `empty($entity->field)` will invoke the magic method `__isset()` – which is the intended use as of the documentation – feeela Jun 01 '16 at 11:44
6

If some info should be made public, define a getter for it. If it's modifiable, add a setter (even better, add a fluent setter!).

API's are cleaner this way, with no magic involved. I don't like magic in my code.

Just my two cents :)

By "fluent setter" I meant one implementing the fluent interface pattern.

xPheRe
  • 2,333
  • 24
  • 33
  • What's a 'fluent setter'? Can you edit your answer and add a link or something? – rjmunro Dec 16 '11 at 11:41
  • 2
    So basically a setter that ends `return $this;` so you can do jQuery-like chaining: `$object->setName('name')->setCode('code');`. Nice idea. It would be good to add this to the setters made by the `orm:generate-entities` tool. – rjmunro Dec 19 '11 at 10:53
2

Yes getter and setter methods are the way to access your data. They are a little cumbersome which is why some people do not like doctrine2 or hibernate. But you only need to do it once for each entity and then they are very flexible to produce the output formatting you are hoping for. You can use the cli to do some of this for you. But when you get them rolling I don't find it to big a deal. Especially since you only do this to the properties you need.

Cheers

Richard
  • 1,162
  • 1
  • 11
  • 19
2

Rather than having seperate getter and setters, or even using the magic functions.. Is there problem with having something like this in the class

public function Set($attrib, $value)
{
    $this->$attrib = $value;    
}

public function Get($attrib)
{
    return $this->$attrib;
}

It makes it much easy to access the attributes and means they be dynamically set from key-pair arrays.. any comments? or alternative suggestions?

konsolenfreddy
  • 9,551
  • 1
  • 25
  • 36
KW1
  • 29
  • 1
1

Doctine 2 provides a [command line tool][1] to generate basic entity classes

use the following to get a basic Entity class definition from your mapping, complete with getter/setter functions for each property:

path/to/doctrine_cli orm:generate-entities --generate-methods=true path/to/entities/

You're still responsible for modifying each getter/setter to ensure that they are the proper datatype, as the getter/setter methods generated for the Entity don't do any type casting/converting.

Mars Redwyne
  • 1,267
  • 13
  • 8