I don't personally feel like redirects should be inside of service classes. As a personal preference, I like my services to be "black boxes" - they should do one thing and do them well. In your case the redirect is coupled to the service and would make your service unworkable in non-http situations. For example, how would you use the service to authenticate a single command line action where you might want credentials passed as command arguments?
The purpose of a controller is to direct application flow - that includes handling redirects based on the results of a service call.
Also, if you have an aggregate service (one that does many things) - remember that your service can inject and use other services if necessary. Every part of a system that you can make smaller and less coupled is a win for the architecture of your project.
I agree with abbajbryant, your service shouldn't know anything about the http layer, or any layer above it (the object owner).
You are almost there though with your current implementation. Using a listener or the Delegation Pattern is nice way to handle this but it needs to be implemented in a way that is flexible with any object that instantiates it. Therefor we can't rely on $listener->actionFails() returning an object that has a method called withErrors (unless specifically docblocked because this is not a static language). You could have Listener->actionFails expect an $errors
parameter which then does what it wants with those errors e.g. pass it to a redirectResponse.
E.g. Controller
<?php
public function postSignin (){
//...
return $this->authenticator->attempt($input , $remember, $this);
}
public function actionFails(MessageBag $errors)
{
return $this->redirector->route('route.name')-withError($errors);
}
Service
<?php
class Authenticator {
//....
public function attempt( $credentials , $remember, ListenerInterface $listener )
{
if ( ! $this->signin_validation->isValid( $credentials )){
$messages = $this->signinValidation->getMessages();
return $listener->actionFails($messages);
}
//...
}
}
One last thing is your listening interface is very generic. Perhaps you need an AuthenticationListenerInterface
instead. Instead of implementing the generic actionFails
method it could implement authenticationFails
. This way each of your services can gain more control over what each listener can listen for and what parameters are passed to those listener methods.
So to answer your questions
Should redirects be in the service or the controller ? Controller
Should I have a different ListenerInterface for each service ? I would, but it depends on the needs of the system.
Is there any other misunderstanding of the pattern in the code ? Your pretty close. It just needs some refining.
Thanks damienadermann & abbajbryant , Nice explanation :D
Sign in to participate in this thread!
The Laravel portal for problem solving, knowledge sharing and community building.
The community