Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Okay, so the goal here is to implement a strategy pattern that can be tested using any sort of automated testing tool.

I've been struggling with this problem conceptually for a few weeks now and I want to clarify it.

So the current setup is something like this:

<?php
// CreateReportUseCase.php
class CreateReportUseCase
{
 protected $factory;
 public function __construct(Factory $factory)
 {
    $this->factory = $factory;
  }

  public function create($params)
  {
    // based on something in the params
    // ask the factory to give us one of the strategy classes

    // for this example, let's assume I'm going to execute
    // a different strategy based on the user role.
    // e.g. Admin
    $strategy = $this->factory->make($params['user_role']);

    // execute the strategy using the data out of the params
    $strategy->execute($params);
  }
}

// Factory.php
class Factory
{
  protected $admin_strategy;
  protected $user_strategy;
  public function __construct(AdminStrategy $admin_strategy, UserStrategy $user_strategy)
  {
    $this->admin_strategy = $admin_strategy;
    $this->user_strategy = $user_strategy;
  }

  public function make($role)
  {
    switch($role)
    {
       case 'Admin':
         return $this->admin_strategy;
         break;
       case 'User':
         return $this->user_strategy;
         break;
    }
  }
}

AdminStrategy implements RoleStrategy
{
  public function execute($params)
  {
    // it's not important what we do here.
    return json_encode($params);
  }
}

Does this look sane to you? How could I improve it? To me, the factory looks like it's a bit awkward since it's just returning some injected dependency instead of instantiating a strategy.

share|improve this question
    
It is a little counterintuitive since you usually call factories to create instances. In your case, your "Factory" is CreateReportUseCase and it doesn't directly create the instances but rather forwards the task to the passed parameter, which kind of defeats the purpose. How about you only use Factory, except with a dynamic way to create an instances, given a map/dictionary of role->strategy? –  Neil Apr 15 at 12:34
    
this is the best compromise I could come up with. if I were to have a dictionary of role->strategy, wouldn't I need to add all of the strategies (which are more numerous then 2) to my use case class? –  Carvefx Apr 15 at 12:59
    
I don't see any strategy pattern in your code. The instance of strategy should be attribute of class where it is used. Having it only locally in method completely defeats the purpose. –  Euphoric Apr 15 at 13:08
    
@Carvefx Your Factory needs to know all strategies in order to create them. It's up to you whether or not you want to be able to add them beforehand. The advantage comes from being able to decouple future calls to Factory and not know what strategy exists for what role. –  Neil Apr 15 at 13:35
    
@Euphoric this is a simplified example, I store it in a property on the usecase on my production code. –  Carvefx Apr 15 at 15:49

1 Answer 1

up vote 0 down vote accepted

In the end, collecting all advice in the comments section, I went with making the Factory an actual Factory, providing it with a dictionary between some parameters like 'User' and a fully qualified namespace + class name.

This way the Factory isn't coupled with the actual strategies, and it actually builds instances instead of just passing an attribute back.

As suggested, the chosen strategy is then placed inside a property on the Context class.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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