Reputation: 1112
In my application, users can sign up for 3 kinds of licenses: A free license that is available for everyone, a subscription license that people have to pay for (managed via an external party) and an invoice licensed, which requires them to contact us.
People can choose their license when registering or later, via their profile. These are two different controllers with different methods.
I have a subscription.manager
service method subscribe(User $user, License $license)
that receives a User and a License entity and handles all the logic of signing the user up for their correct license.
Based on what license the user selected, the result can vary:
The last option is a simple RedirectResponse
, but based on whether they registered or simply changed their existing license subscription, I want to display different pages.
What would be the best way to handle this?
At the moment, I do this:
$response = $this->get('subscription.manager')->subscribe($user, $license);
switch (true) {
case $response instanceof SuccessResponse:
return $this->redirect('success_url');
break;
case $response instanceof RequestedResponse;
return $this->redirect('requested_url');
break;
case $response instanceof RedirectResponse:
return $response;
break;
default:
throw new \Exception('Response not recognized');
}
SuccessResponse
and RequestedResponse
are simple classes I created and hold little to no info themselves, they purely indicate what happened in the method.
This allows me to copy paste this block and simply switch up the success and requested urls. However, this feels not very optimal. Are there better ways of doing this?
I guess I could create the success and requested responses (whether a Redirect or Render) and pass those to the service method. But that feels like I'm violating the Single responsibility principle.
Upvotes: 1
Views: 61
Reputation: 48883
I don't think your subscription manager should concern itself at all with redirect and stuff. So as a first step I would do:
$result = $this->get('subscription.manager')->subscribe($user, $license);
switch ($result) {
case 'ProcessedFreeLicense':
return $this->redirect('success_url');
break;
case 'ProcessedInvoicedLicense';
return $this->redirect('requested_url');
break;
case 'ProcessedSubstrictionLicense':
return $response;
break;
default:
throw new \Exception('Response not recognized');
}
That still leaves you with a switch statement in the controller and some repetitive code. You could move the switch statement to a base controller class.
However, I would consider dispatching a ProcessedLicense event and then letting a listener decide what to do for each sort of license. Take a look at the development version of FOSUserBundle.RegistrationController for a working example but basically:
$result = $this->get('subscription.manager')->subscribe($user, $license);
$event = new ProcessedLicenseEvent($user,$license,$results);
$dispatcher->dispatch(LicenseEvents::PROCESSED_LICENSE, $event);
return $event->getResponse();
So now all of the logic associated with what to do after processing a license can but tucked inside one or more listeners. It can be changed as needed without impacting your controllers.
=====================================================================
Just thought I would add a third approach for completeness. This approach is quite common in some C#/Java based applications. It's a "fire and forget" approach in which the command does not return a value nor does the controller expect one.
// This is the Command. It does not return a value.
$this->get('subscription.manager')->subscribe($user, $license);
// Always just go here
return $this->redirect('user_license_status_page');
Clearly we simplified our command object since it no longer needs to return a value. And we got rid of the shared switch statement. The licence_status controller can show the user what happened and take further action if necessary.
Upvotes: 1