2

I am building a quiz app with Symfony2. For this example, let's say I have two question Entities both extending a Question abstract class

  • TrueFalseQuestion
  • MultipleSelectQuestion

I want to build services to check if a user response is correct or not. I will have TrueFalseQuestionChecker and a MultipleSelectQuestionChecker.

What is the better way to choose which one of the service to load ? In my controller I could do:

if($question instance of MultipleSelectQuestion::class){
    $checker = $this->get('multiple_select_question_checker');
} else if($question instance of TrueFalseQuestion::class){
    $checker = $this->get('true_false_question_checker');
}

$checker->verify($question);

But I find it very "ugly" to do this in my controller because I will have a list of like 10 question types and I will have to do this for a serializer service, for the answer checker service and maybe for other services. Is there a proper way to deal with an association between a service and an Entity.

I'm thinking of implementing my own annotation, something like that:

/**
* @MongoDB\Document
* @Question(serializer="AppBundle\Services\TrueFalseQuestionSerializer", 
* responseChecker="AppBundle\Services\TrueFalseQuestionChecker"
*/

public class TrueFalseQuestion extends Question 
{
...
}

Am I missing something already include in Symfony ? Or is my idea of doing one service by question type bad ?

Edit: Working solution thanks to @Tomasz Madeyski

src/AppBundle/Document/TrueFalseQuestion

/**
 * Class TrueFalseQuestion
 *
 * @MongoDB\Document(repositoryClass="AppBundle\Repository\TrueFalseQuestionRepository")
 * @QuestionServices(serializer="app.true_false_question_serializer")
 */
class TrueFalseQuestion extends Question
{ 
...

src/App/Bundle/Annotation/QuestionServices.php

<?php
namespace AppBundle\Annotation;

/**
 * @Annotation
 */
class QuestionServices
{
    private $checker;

    private $serializer;

    public function __construct($options)
    {
        foreach ($options as $key => $value) {
            if (!property_exists($this, $key)) {
                throw new \InvalidArgumentException(sprintf('Property "%s" does not exist', $key));
            }

            $this->$key = $value;
        }
    }

    public function getService($serviceName)
    {
        if (isset($this->$serviceName)) {
            return $this->$serviceName;
        }
        throw new \InvalidArgumentException(sprintf('Property "%s" does not exist', $serviceName));

    }

}

src/AppBundle/Services/QuestionServicesFactory.php

<?php

namespace AppBundle\Services;

use Doctrine\Common\Annotations\Reader;
use ReflectionClass;
use AppBundle\Annotation\QuestionServices;
use Symfony\Component\DependencyInjection\ContainerInterface;

class QuestionServicesFactory
{
    const SERVICE_SERIALIZER = 'serializer';
    const SERVICE_CHECKER = 'checker';

    public function __construct(Reader $reader, ContainerInterface $container)
    {
        $this->reader    = $reader;
        $this->container = $container;
    }

    /**
     * @param string $questionClass
     * @param        $serviceName
     *
     * @return object
     * @throws \Exception
     */
    public function getQuestionService($questionClass, $serviceName)
    {
        if (!class_exists($questionClass)) {
            throw new \Exception(sprintf("The class %s is not an existing class name", $questionClass));
        }

        $reflectionClass  = new ReflectionClass($questionClass);
        $classAnnotations = $this->reader->getClassAnnotations($reflectionClass);

        foreach ($classAnnotations as $annotation) {
            if ($annotation instanceof QuestionServices) {
                $serviceName = $annotation->getService($serviceName);

                return $this->container->get($serviceName);
            }
        }
        throw new \Exception(sprintf("Annotation QuestionServices does not exist in %s", $questionClass));
    }
}

service declaration

   <service id="app.question_services_factory"
             class="App\Services\QuestionServicesFactory">
        <argument type="service" id="doctrine_mongodb.odm.metadata.annotation_reader"/>
        <argument type="service" id="service_container"/>
    </service>

in controller

    $questionServiceFactory = $this->get('app.question_services_factory');
    $questionSerializer     = $questionServiceFactory->getQuestionService(
        TrueFalseQuestion::class,
        QuestionServicesFactory::SERVICE_SERIALIZER
    );
iBadGamer
  • 566
  • 6
  • 22

3 Answers3

1

I suggest to you to use a class to get the service something like this:

$serviceClass = new ServiceClassSearcher();
$serviceName = $serviceClass->getServiceName($question);
$checker = $this->get($serviceName);

Inside your class you can make something like this:

class ServiceClassSearcher
{
   private $service = [
      'MultipleSelectQuestion' => 'multiple_select_question_checker',
      'TrueFalseQuestion' => 'true_false_question_checker'
   ];

   public function __construct()
   {
   }       

   public function getServiceName($serviceInstance)
   {
       foreach ($this->service as $instance => $serviceName) {
           $className = instance . '::class';
           if($question instance of $className){
               return $value;
           }
       }

       return null; //is better to throw an exception for a class not found
   }
}
Alessandro Minoccheri
  • 35,521
  • 22
  • 122
  • 171
1

This is basically a bit opinion based question but: @Allesandro Minoccheri answer will work but it has one downside: adding new type of question and question checker will require modifying ServiceClassSearcher for each new pair - so this is a bit against SOLID rules.

I think that Question should know what type of checker should check it, so ideally I would inject QuestionChecker into Question. Since Question is an entity and DI is not possible here I would say that using annotation to specify what type of checker is responsible for checking given question is a good way to do it. Once you have annotation you only need to add class which will parse it and get instance of your checker. This way, once you want to add new type of question you don't need to modify any of existing code.

Also I would suggest to use question checker service name instead of class name as it will be easier to use.

A class to parse annotation and get question checker can look something like:

use Doctrine\Common\Annotations\Reader;

class QuestionCheckerFactory
{
    public function __construct(Reader $reader, ContainerInterface $container)
    {
        $this->reader = $reader;
        $this->container = $container;
    }

    public function getQuestionChecker(Question $question)
    {
        $reflectionClass = new ReflectionClass(get_class($question));
        $classAnnotations = $annotationReader->getClassAnnotations($reflectionClass);

        foreach($classAnnotations as $annotation) {
            if ($annotation instanceof \Your\Annotation\Class) {
                //now depending on how your Annotation is defined you need to get question checker service name
                $serviceName = ...
                return $this->container->get($serviceName);
            }

        }

        throw new Exception();

    }
}

Note: I wrote this code out of my head so there might be some parsing errors, but the general idea is here.

You can check this blog post about using custom annotations

Tomasz Madeyski
  • 10,742
  • 3
  • 50
  • 62
  • It could be a good way too, I would like to view It implemented a little bit – Alessandro Minoccheri Aug 08 '17 at 13:42
  • Yes it is an opinion based question ! When you say 'Question' are you referring to Entities extending Question ? I will have SomeInteractionQuestion extends Question with an annotation in SomeInteractionQuestion specifying the service name to load ? And in my controller I will read the annotations of my question class and load the correct service based on the annotation ? – iBadGamer Aug 08 '17 at 14:01
  • guys, check my edit. @iBadGamer: yes, by saying Question I refer to Entities extending abstract Question class – Tomasz Madeyski Aug 08 '17 at 14:08
  • Ok I like this solution and will git it a try. I never used custom annotations but this is the opportunity to do so. Thank you guys ! I will get back to you to confirm it is working as expected – iBadGamer Aug 08 '17 at 14:14
  • I have edited my question with the working solution, thank you ! In your code you only have to replace $classAnnotations = $annotationReader->.. by $classAnnotations = $this->reader->... – iBadGamer Aug 10 '17 at 08:51
0

I believe you are looking for is the Factory Pattern

The factory:

class QuestionCheckerFactory
{
    public function __construct() // inject what you need to created the services
    {
    }

    public function get(Question $q)
    {
        if($q instance of TrueFalseQuestion)
        {
            return new TrueFalseQuestionChecker(...);
        }

        throw new \Exception('Not implemented ...');
    }
}

$checker = $this->get('question.checker.factory')->get($question);
$checker->verify($question);
RVandersteen
  • 2,067
  • 2
  • 25
  • 46
  • But where will I put my services dependencies ? I find it weird to instantiate a service with 'new TrueFalseQuestionChecker(...)' instead of getting it from the service container – iBadGamer Aug 08 '17 at 14:05
  • You could inject the container in the QuestionCheckerFactory – RVandersteen Aug 08 '17 at 14:06
  • again, adding new type of `Question` requires modyfing `QuestionCheckerFactory` which is against SOLID rules – Tomasz Madeyski Aug 08 '17 at 14:09
  • Wich one ? What you've written above (with annotations) is just an automated version of what's written here (or what am I missing ?) – RVandersteen Aug 08 '17 at 14:12
  • Open close principle. Yes, it is automated version of what you've written and that's why it has an advantage: when you want to add new Question / QuestionChecker pair only thing you need to do is add them, without modyfing QuestionCheckerFactory. In your case you may end up with list of dozen `if`s – Tomasz Madeyski Aug 08 '17 at 14:14
  • Yes I see, however. I believe this is beautifully put https://stackoverflow.com/a/7876804/4640433. – RVandersteen Aug 08 '17 at 14:18
  • Another way around it could be to use the Strategy Pattern instead – RVandersteen Aug 08 '17 at 14:23