inklabs / kommerce-core

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

Services should return Entities, not Views #5

Closed pdt256 closed 9 years ago

pdt256 commented 9 years ago

At the present time, Services are are returning View objects. Originally this was desired in order to not expose Entities that had the ability to change state or produce database queries through lazy loaded proxies.

Service\Product::find()

    public function find($id)
    {
        $product = $this->productRepository->find($id);
        if ($product === null) {
            return null;
        }
        return $product->getView()
            ->withAllData($this->pricing)
            ->export();
    }

This results in tests that are much more difficult to manage in case of null objects. This extra test gets duplicated throughout the codebase.

test\Service\ProductTest::testFindMissing()

    public function testFind()
    {
        $product = $this->productService->find(1);
        $this->assertTrue($product instanceof View\Product);
    }
    public function testFindMissing()
    {
        $this->productRepository->setReturnValue(null);
        $product = $this->productService->find(1);
        $this->assertSame(null, $product);
    }

Returning just the Entity will clean up the tests and make for a simpler Service Layer.

Service\Product::find():

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

test\Service\ProductTest::testFind()

    public function testFind()
    {
        $product = $this->productService->find(1);
        $this->assertTrue($product instanceof Entity\Product);
    }
pdt256 commented 9 years ago

In addition, we remove the dependency on Lib\Pricing in the Services.