1
\$\begingroup\$

I use Laravel, but hopefully this doesn't matter. I have a user and user's state. Say, simple integer or a string. I want to run some logic depending on user state in my service class.

So, the first approach is pretty simple:

<?php

class UserStateManager
{
    public function __construct(UserRepository $userRepository)
    {
        $this->userRepository = $userRepository;
    }

    public function handle(UserRepository $userRepository)
    {
        $user = $this->userRepository->find(1);
        switch ($user->state) {
            case 'new':
                $this->handleUserNewState();
                break;
            case 'banned':
                $this->handleUserBannedState();
                break;
            // ...
        }
    }

    private function handleUserNewState()
    {
        // Do some logic
        // Here we have access to userRepository which was injected in UserStateManager constructor
    }

}

Pay attention that I injected UserRepository in the constructor.

So, using this approach I keep UserStateManager dependencies injected, without creating them manually.

But now I want to put every state handler to a separate class. Like, to have UserNewState class with handle method.

The problem is that in this case:

public function handle(UserRepository $userRepository)
    {
        $user = $this->userRepository->find(1);
        switch ($user->state) {
            case 'new':
                (new UserNewState)->handle();
                break;
            case 'banned':
                (new UserBannedState)->handle();
                break;
            // ...
        }
    }

if I create UserNewState object I cannot inject dependencies into this class since I manually create it using new operator. Passing lots of dependencies in the constructor (like new UserNewState($this->userRepository, ...)) is not an option, ugly solution.

So how to deal with this case? Because it's very common: on the one hand I can separate logic into several methods inside one class, on the other hand I cannot separate logic into many classes.

The problem is dependency injection and creating these class manually. And I reckon that DI is a good thing.

Of course I can create dependencies inside UserNewState class, directly in the constructor:

public function __constructor() {
    $this->userRepository = new UserRepository;
}

but this is not testable. Even though I don't test my apps, I just want to follow best practices.

\$\endgroup\$
  • \$\begingroup\$ I can write you an essay of an question but its symfony based.Its an architecture thats supper easy to follow.And it covers everything from DiC to some other patterns.Just want to make sure that you are okay with symfony @Victor \$\endgroup\$ – DaAmidza Dec 3 '17 at 21:41
  • \$\begingroup\$ Well, it shouldn't depend much on framework I think? I haven't used Symfony (except, probably, some components which are used in Laravel). But I'd of course appreciate any information regarding this issue since I come across this pretty often!:) \$\endgroup\$ – Victor Dec 4 '17 at 0:57
1
\$\begingroup\$

Without knowing exactly what those classes are supposed to do, I assume it's supposed to work something like (new UserNewState( $this->userRepository ))->handle( $user );. That doesn't look that bad to me.

What other dependencies do you envision your separate State classes are going to need besides the repository?

But perhaps something like a UserStateHandlerFactory is something you could be looking for?

class UserStateHandlerFactory
{
  private $userRepository;

  public function __construct( UserRepository $userRepository ) {
    $this->userRepository = $userRepository;
  }

  public function create( User $user ) {
    switch ($user->state) {
        case 'new':
            return new UserNewState( $this->userRepository );
            break;
        case 'banned':
            return new UserBannedState( $this->userRepository );
            break;
        // ...
    }
  }
}

class UserStateManager
{
  private $userRepository;
  private $handlerFactory;

  public function __construct( UserRepository $userRepository, UserStateHandlerFactory $handlerFactory )
  {
    $this->userRepository = $userRepository;
    $this->handlerFactory = $handlerFactory;
  }

  public function handle()
  {
    $user = $this->userRepository->find(1);
    $this->handlerFactory->create( $user )
                         ->handle( $user );
  }
}

However, I don't know if that makes things much cleaner though. It just looks like another layer of indirection; hiding away, what ultimately has to be done anyway: pass dependencies into the handlers.

\$\endgroup\$
  • \$\begingroup\$ Yep, I use it excatly as you stated ((new UserNewState( $this->userRepository ))->handle( $user );) I thought about factory. Like to create a factory class, pass in its constructor aaaall dependencies, and then create objects manually and pass dependencies in the factory class. Like you have in "create" method. Well, for now I prefer to create objects manually without factory, since I don't write tests, but in the future I'll probably refactor my code using factory. However I'm not sure if it's the only approach. Thanks for you reply though! \$\endgroup\$ – Victor Dec 4 '17 at 1:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.