CODEWITHSUNDEEP
phpdesign-patterns
Łukasz Zaroda
Łukasz Zaroda

Reputation: 767

PHP: Design pattern for managing types of entity

I have User entity. I would like to have multiple "types" of this entity, with different managers and repositories. All User entities of all types would share only UserInterface. Now, I'm looking for a good method of organizing everything. First thing that came to my mind is to create something like this:

interface UserTypeManagerInterface
{
  public function addUserType($name, RepositoryInterface $repository, ManagerInterface $manager);
  public function hasType($name);
  public function getRepository($type);
  public function getManager($type);
}

Then in places where I would like to manage multiple types of User at once, I would inject this, and in places where I would like to manage a specific type of user, I could inject only specific repository and manager objects, for its type.

Seems like a pretty clean approach, but at the same time, when I would like to create a test for a class using UserTypeManager I would need to mock UserTypeManager, then this mock would need to return other mocks (repository and manager).

This is doable of course, but it made me thinking if this can be avoided. The only other thing that I can think of, which would allow for avoidance of above complexity during testing, would be something like this:

interface UserTypeManagerInterface {
    public function addUserType($name, RepositoryInterface $repository, ManagerInterface $manager);
}

/**
 * My class managing multiple types of user.
 */
class ManageMultipleTypesOfUsers implements UserTypeManagerInterface {
    // ...
}

So I would just add all repositories and managers to all classes implementing UserTypeManagerInterface interface. So objects would use directly what was given to them.

This way testing would be much cleaner, because I would need to mock only the one manager and one repository to test class ManageMultipleTypesOfUsers, but this feels sooo much like over-engineering. ;)

Is any middle-ground here even possible?

Upvotes: 6

Views: 1644

Answers (1)

bwoebi
bwoebi

Reputation: 23777

I can not give a definitive answer on this as it is a trade-off.

As far as I see User is a pure value object? That's a good start. That means it's trivial to manipulate without side-effects.

Now, consider these variables which shall influence your design:

  • Is the interface just one method? [Or are all its methods trivial wrappers for one method?]
  • Is the interface meant to have the users of your library define custom implementations?
  • Do the implementations just add assertions (e.g. the interface requires a method delete(User $user) - only Admins are allowed, other implementations just throw, basic permission checks) or use vastly different code?
  • How much over-engineered vs untestable mess do we have?

So, how do these variable influence our decision?

  • If you ever only have one [smaller] central method, then interfaces plus classes are likely to be overkill and tend to litter your codebase with many small files. Often passing a Closure around also achieves the same effect.
  • Apart from when simple Closures/callables are enough, public API shall always be interfaces.
  • When implementations just add assertions, you typically are better off with one single class dispatching a request to the access control manager [which reads/caches the permissions from database].
  • over-engineered vs untestable: 50 files with each 3 lines of different code are better merged into a single file with 300 lines.

As I do not know what the requirements will be, I cannot give you an ideal solution. The (second) solution you proposed is fine for the "over-engineered" case.

A more moderate solution would be the functional approach:

function addAdminUser($name, RepositoryInterface $repository, ManagerInterface $manager) { /* ... */ }
function addNormalUser($name, RepositoryInterface $repository, ManagerInterface $manager) { /* ... */ }
// you even can pass this around as a callable ($cb = "addAdminUser"), or (pre-binding):
$cb = function($name) use ($repo, $mgr) { addAdminUser($name, $repo, $mgr); };

Or radically (if normal user is a subset of admin user addition) [as long as the function itself is side-effect free and its callers not going to be too hard to test]:

function addUser($name, $type, RepositoryInterface $repository, ManagerInterface $manager) {
    /* ... common logic ... */
    if ($type == IS_ADMIN_USER) { /* ... */ }
}

If that's too radical, also possible to inject a callback into addUser:

function addUser($name, $cb, RepositoryInterface $repository, ManagerInterface $manager) {
    $user = new User;
    /* ... common logic ... */
    $cb($user, $repository, $manager);
}

Yes, the functional approach is maybe not as testable (i.e. you won't be able to mock the function if you call it directly (without passing as callable)), but it may bring you a huge gain in readability. Saving levels of trivial indirection may make your code easier to analyze. I'm emphasizing the may, as removing too much indirection may achieve the opposite effect. Find the middle ground as applicable to your application.

TL;DR: PHP provides you with a more functional approach, but less testable. Sometimes that's a fine trade-off, sometimes not. It depends on your concrete code where the middle ground lies.

P.s.: The only thing which really smells a bit to me in your second proposal, is having RepositoryInterface $repository, ManagerInterface $manager in the addUserType method signature. Are you sure it's not part of the constructor?

Upvotes: 1

Related Questions