Should entity properties be protected or public?

[NotionCommotion]

abstract class Entity
{
    public function __get($property) {
        if (property_exists($this, $property)) return $this->$property;
        else throw new NodeException("Property '$property' does not exist");
    }

    public function __set($property, $value) {
        if (!property_exists($this, $property)) throw new Exception("Property '$property' is not allowed");
        $this->$property = $value;
        return $this;
    }
}

class Node extends Entity
{
    protected $a, $b;

    public function __construct(array $data, array $keys=[]) {
        if($keys) {
            foreach($keys as $key) {
                if (isset($data[$key])) throw new Exception("Element for '$key' not provided");
                else $this->__set($property, $value);
            }
        }
        else {
            foreach($data as $property => $value) {
                $this->__set($property, $value);
            }
        }
    }

}

class Collection extends Entity
{
    protected $stack=[];
    private $originalStack;

    public function __construct(array $nodes=[]) { //$nodes is an array of Node objects
        $this->stack=$nodes;
        $this->originalStack=array_merge(array(), $nodes);    //Arrays not passed by reference but the objects are!
    }

    public function getPositionChanges($prop){
        return array_udiff_assoc($this->stack, $this->originalStack,
            function ($stack, $originalStack) {
                return is_numeric($stack->$prop) && is_numeric($originalStack->$prop)
                ?$stack->$prop - $originalStack->$prop
                :strcmp($stack->$prop, $originalStack->$prop);
            }
        );
    }

    public function getAll(){
        $this->stack;
    }

    public function getByPosition(int $position){
        $this->stack[$position];
    }

    public function getPropertyByPosition(int $position, $property){
        return $this->stack[$position]->$property;  //Uses __get
    }

    public function add(Node $elem){
        $this->stack[]=$elem;
    }

    public function delete(int $position){
        if(!isset($this->stack[$position])) throw new CollectionException('Index does not exist');
        unset($this->stack[$position]);
        $this->stack=$stack[]=array_values($this->stack);
    }

    public function move(int $initialPosition, int $finalPosition){
        if(!isset($this->stack[$initialPosition]) || !isset($this->stack[$finalPosition])) throw new CollectionException('Index does not exist');
        $node=$this->stack[$initialPosition];
        unset($this->stack[$initialPosition]);
        array_splice($this->stack, $finalPosition, 0, $node);
        $this->stack=array_values($this->stack);
        //?? return $this->stack;
    }

    public function update(int $position, $property, $value){
        if(!isset($this->stack[$position])) throw new CollectionException('Index does not exist');
        $this->stack[$position]->$property=$value;  //Uses __set
    }
    public function getChanges(){
        //Not complete
    }

}

class MyEntity extends Entity
{
    protected $collections=[];
    protected $foo, $bar;

    public function __construct($properties, array $collections=[]) {
        $this->collections=$collections;
        $this->addArrayToThis($properties);
    }

    protected function addArrayToThis($properties) {
        foreach($properties as $name=>$value){
            $this->$name=$value;
        }
    }

}

$collections=[];
//Simulate getting data from the database
for ($i = 1; $i <= 10; $i++) {
    $nodes=[];
    for ($j = 1; $j <= 10; $j++) {
        $nodes[]=new Node(['a'=>'A','b'=>'B']);
    }
    $collection[]=new Collection(['x'=>'X'],$nodes);
}
$myentity=new MyEntity(['foo'=>123,'bar'=>321] ,$collection);
echo(json_encode($myentity));
var_dump($myentity);

I think the answer is protected, but doing so is causing me some issues.  Ultimately, I wish to convert the entity to JSON and send it to the client, but doing so will not work as the properties are protected.  Maybe need a function like getJson() which returns json_encode($this)?  Or maybe getThis() which returns $this?  How does one go about doing this?

By the way, using entity classes is rather new to me.  Does it sound like I am on the right path?

Thanks

PS.  Sorry for the text at the bottom.  Added the code before adding the narrative, and cannot seem to add text at the top.

[NotionCommotion]

Ah ha, http://php.net/manual/en/class.jsonserializable.php to the rescue!

[ignace]

Entity fields should be private, not protected (extend is not a good reason), and certainly not public. Instead use getters/setters.

Converting the object to JSON should be left to a separate class, preferably this new class should define an interface. Example given:

<?php

interface NodeMapper {
  public function map(Node $node);
  public function reverseMap($output): Node;
}

final class JsonMapper implements NodeMapper {
  public function map(Node $node) {
    // bla bla
    return json_encode($nodeMapArray);
  }
  
  public function reverseMap($output): Node {
    $foo = json_decode($output);
    
    return new Node(..);
  }
}

You can take it a step further:

<?php

final class NodeToJsonMapper {
  function map(Node $node): StdClass {
    // bla bla
    return json_encode($nodeData);
  }
}

final class JsonToNodeMapper {
  function map(StdClass $json): Node {
    // bla bla
    return new Node(..);
  }
}

 

Edited May 31, 2018 by ignace

[maxxd]

I'm with ignace on this one - object properties should be private until they absolutely must be made public (which is hopefully never). Protected is better than public, but most patterns these days prefer composition over inheritance so protected is kinda pointless.

Having said that, I'm a big fan of the encapsulation aspect of OO so I may be a bit biased. I think Google at one point released a white paper on writing speedier code and one of the things they recommended was eschewing getters and setters in favor of public properties. No, but thankyouverymuch.

[NotionCommotion]

Thank you ignace and maxxd,

I guess I understand the reason for never (which is never a good word) public.  But why not protected.  Are you implying, ignace, that entity classes should never be extended?

Part of my conundrum is the necessity of these entity classes in the first place.  Yes, for some cases, I see some benefit, but not always and often not much. 

I have never used an ORM and I seem to think more SQLish than Objectish, and wonder if this is contributing to my lack of seeing their compelling benefits.

[ignace]

Quote

Are you implying, ignace, that entity classes should never be extended?

No I am not, though it's a good thing to always start from final class, as it will make you think harder about your code.

That said, I extend my entity classes every time there's a need. Protected properties and inheritance are two separate things anyway. A protected property is a "public" property in disguise, just think about that for a second. Why would a child have privileged access to "private" information?

If there is a need to give privileged information to a child then you can still resort to a protected method. This will ensure the data remains encapsulated. Is available to the child as a sort of contract (guaranteed access to the data). And allows the parent to change properties anyway they want (eg from a string to an object).

[NotionCommotion]

On 5/31/2018 at 9:44 AM, ignace said:

Converting the object to JSON should be left to a separate class, preferably this new class should define an interface.

Thanks again ignace,

I had originally thought that the JsonSerializable interface was my perfect solution, and that it should belong with the entity.  The problem I found, however, was it was not flexible for multiple views.  Is this your primary reason for stating that it converting the object to JSON should be left to a separate class?

[ignace]

Yes, preferably it should always be the responsibility of another class unless the domain warrants the use of JsonSerializable. For example, the object is stored/read from a JSON store.

[maxxd]

IMO, a series of serialization objects is a good example of when using inheritance and protected properties is justified. Have a base serialization factory class that then calls child classes for JSON serialization, normal serialization, XML output, MLS XML, MathML, SuperSpecialUnicorn serialization (I'm making up examples, obviously), whatever. Each of the child classes can then read and modify any protected property on the parent factory class and as long as you program to that parent factory's API you really don't have to care which it is.

Edited June 2, 2018 by maxxd