reshadman / laravel-repository-response

Make a contract on the response of your Repository class methods.
GNU General Public License v2.0
4 stars 2 forks source link

may resemble the pattern . not sure ... #1

Closed hasangilak closed 9 years ago

hasangilak commented 9 years ago

<?php

class A { public function delete() { echo "deleted"; } }

//////

<?php

class B { protected $doomed = ['delete']; protected $A; public function __construct(A $a) { $this->A = $a; }

public function refactor()
{
    $this->A->delete();
}

public function __call($method, $args)
{
    if(! in_array($method ,$this->doomed))
    {
        return call_user_func_array([$this->A, $method], $args);
    }
    throw new Exception(
        "No method with name [{$method}] found in the entity and the related eloquent model."
    );
}

}

//////

<?php

include "A.php"; include "B.php";

$b = new B(new A);

$b->delete();

reshadman commented 9 years ago

Yep, it works , but doesn't solve the problem of calling methods from friend clsses. Let me explaing the workflow :

In current repository pattern implementation using eloquent models, the model is returned as responses, so suppose the below code :


$user = $this->userRepo->findById($id);

$user->delete(); // This works perfectly in your implmentation

// Now we have the user  entity and we want to delete it using eloquents delete method which
// Raises an event, so our User::deleteing() closure is called, and for example we can send him 
// a confiramtion email.

class UserRepo {
     function deleteByUserEntity(User $user)
     {
           return $user->delete(); // your code blocks this, too
          // to make ourselves able to define who can call the delete method on the eloquent model
          // (explicitly) we have to implement the Friend class approach which is hardly(badly) possible
          // with debug_back_trace function .
          // another solution would be adding a getModel() method to the "B" class which is only called 
          // inside the repository. but it is not explicit again, you can get the model in any place outside the
          // repository by using $b->getModel()->delete(); which touches our database
          // from outside of our repository contract and breakes the pattern.
     }
}
hasangilak commented 9 years ago

in fact userRepo must be instance of B class not A .

reshadman commented 9 years ago

Yeah, but doesn't limit the external use any way. beacuse the wrapper doesn't have any control on _Who is using the Eloquent model_. lets make it simple :

hasangilak commented 9 years ago

yeah your approach block things from very beginning , no doubt about it even before issuing this repo. a question, as a developer is possible to do not adhere the pattern ? with implementing an interface i can make old class model ... i conclude that blocking matters only in this intent view points .

reshadman commented 9 years ago

Absolutely, the purpose of the repo was only to prove that it is possible to implement the full pattern using an ORM like eloquent. There is no need to do the explicit checking.

Adding a UserEntityContract and Implementing it by the Eloquent Model solves our problem, by this we can switch our Concrete implmenetation (From Eloquent to Doctrine) and also have the benefits of easier testing. Wrapping each model inside an Entity has huge overhead for Collection responses, each Collection item should be wrapped by an Entity and also if there is a Relationship it should be checked, too.

hasangilak commented 9 years ago

if we need an interface to bypass the contract ,again this makes me feels like over engineering . i mean bypassing is an issue here by the way. lets block things and passing right object or class ... (last comment)

reshadman commented 9 years ago

Usin EntityContracts (Inerfaces) is not the scope of this packages. and also they can be easily implemented

<?php

interface UserEntityContrac {
        public function getId();
}

class UserModel extends \Eloquent implements UserEntityContract {
       protected $table = 'users';

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

class EloquentImageRepo {
       public function getUserImage(UserEntityContract $user)
       {
            if($user instanceof 'Eloquent') return $user->images(); // A collection of image models

            return $this->buildEntityCollection(DB::table('images')->where('user_id', $user->getId())->get());
       }
}

class SimplePdopRepo {
       public function getUserImages(UserEntityContract $user)
       {
               $query = 'SELECT * from images where user_id = ? limit 0,1000';

                return $this->buildEntityCollection(DB::getPdo()->prepare($query)->execute([$user->getId()]));
       }
}

There should be no problems when using an EntityContract to interact with repository. but still the problem of not being able to restrict calls on eloquent model exists.