inklabs / kommerce-core

PHP shopping cart core platform
https://kommerce-laravel-demo.jamieisaacs.com/
Apache License 2.0
65 stars 14 forks source link

Views (plain value objects) have a confusing name #4

Closed pdt256 closed 8 years ago

pdt256 commented 9 years ago

These classes format the Entities as simple objects with public class member variables. View: View\Product Entity: Entity\Product

Currently the name of the View module collides with the Model–View–Controller (MVC) concept. In the case of Kohana, a View class already exists and causes a name collision when using Zen Kommerce Core.

In addition, these Views are currently a Builder Pattern in hiding. This needs to change in order to easily generate a plain old PHP object (with public access variables) from an entity; while actually using a build() method instead of the current export().

$product = new Entity\Product;
$plainProduct = $product->getView()
    ->withTags()
    ->export();
$this->set('product', $plainProduct);

The end goal is to be able to pass this POPO (plain old PHP object) to a template:

<h1><?=$product->name?></h1>
Total Tags: <?=count($product->tags)?>

This is preferred to sending the existing Entity that could change state or execute DB queries via a Doctrine proxy object (e.g., a call to getTags() could execute a DB query via the lazy loaded proxy relationship). This is not something we desire in our templates.

<h1><?=$product->getName()?></h1>
Total Tags: <?=count($product->getTags())?>
pdt256 commented 9 years ago

Voting for a new name for Views is now open.

pdt256 commented 9 years ago

Current suggestions:

bryan-maxwell commented 9 years ago

I see a problem with the way the View construct is implemented. It knows enough about the Entity such that the Entity and View have to be modified in unison to make the properties available in the View.

Perhaps one solution would allow the Entity to return a current-state version of itself as a plain object without the need for an additional View object named the same as the Entity. The entity could implement a method called getObject(), getPlain(), maybe even a buildObject() method could handle this.

I liken it to "flattening" the object so it's current-state exists in properties and is no longer able to executing methods within the Entity itself -- rather the properties or data are accessible.

bryan-maxwell commented 9 years ago

We could use an "EntityFlattener" that returns an instance of an Entity as a flattened object taking public properties and returning them.

class Foo {
    public $arg = 'shaz';
    public $argTwo = 'bot';

    // not flattened by default
    protected $bar  = 2;
    private   $baz  = 3;

    public function __construct() {
        $this->arg = 'foo';
        $this->argTwo = 'bar';
    }
}

class EntityFlattened {

    public function __construct($object)
    {
        $reflector = new ReflectionClass($object);
        $flattenedObject = new stdClass;
        $objectProperties = $reflector->getProperties(ReflectionProperty::IS_PUBLIC);

        foreach ($objectProperties as $property) {
            $this->{$property->getName()} = $property->getValue($object);
        }

        return $this;
    }

}

var_dump(new EntityFlattened(new Foo));

/*
Output:
object(EntityFlattened)#1 (2) {
  ["arg"]=>
  string(3) "foo"
  ["argTwo"]=>
  string(3) "bar"
}
 */

The biggest hurdle I see is that any type hinting that's forced on objects down the line will fail unless they require EntityFlattened.

pdt256 commented 9 years ago

Data Transfer Object - e.g. ProductDTO

pdt256 commented 8 years ago

The new EntityDTO and EntityDTOBuilder classes are complete. Next steps are to remove the View files.