Skip to main content
Code Review

Return to Revisions

2 of 3
deleted 28 characters in body
Kid Diamond
  • 2.6k
  • 17
  • 35

Any improvements for my LoginController class?

This is probably going to be my final question regarding controllers. I feel like I have a good enough understanding of them now and am able to write them cleanly.

I've recently adapted a less error prone way of communicating with the corresponding View. But I would still like to know if there is anything I could further improve about this LoginController class?

namespace Controller;
use View\LoginView;
use Http\HttpRequest;
use Controller\Controller;
use Model\Application\AntiCsrfService;
use Model\Application\AuthenticationService;
use Model\Application\FormValidationService;
class LoginController extends Controller
{
 private $antiCsrfService;
 private $formValidationService;
 public function __construct(
 HttpRequest $httpRequest,
 AuthenticationService $authenticationService,
 LoginView $loginView,
 AntiCsrfService $antiCsrfService,
 FormValidationService $formValidationService
 ) {
 parent::__construct($httpRequest, $authenticationService, $loginView);
 $this->antiCsrfService = $antiCsrfService;
 $this->formValidationService = $formValidationService;
 if ($this->authenticationService->findLoggedInUser()) {
 $this->view->setIsUserLoggedIn(true);
 $this->view->index();
 }
 }
 public function index()
 {
 $this->view->setFormValueOf('csrfToken', $this->antiCsrfService->getToken());
 $this->view->index();
 }
 public function submit()
 {
 $token = $this->httpRequest->findParameter($this->antiCsrfService->getToken()['name']);
 if ($this->antiCsrfService->isTokenValid($token)) {
 $isLoginFormValid = $this->formValidationService->isLoginValid(
 trim($this->httpRequest->findParameter('email')),
 $this->httpRequest->findParameter('password')
 );
 // Only attempt to log the User in when the form has no validation errors.
 if ($isLoginFormValid) {
 $user = $this->authenticationService->loginUser(
 trim($this->httpRequest->findParameter('email')),
 $this->httpRequest->findParameter('password'),
 $this->httpRequest->findParameter('remember') ? true : false
 );
 }
 // Two type of errors in one statement: failed login and form validation (DRY).
 // ↓ Either one would evaluate to true, but never both.
 if (!$isLoginFormValid || !$user) {
 // If login form inputs are valid, login has failed.
 // If login form inputs are invalid, login has not failed.
 // Logging in is not attempted when the login form inputs are invalid.
 $this->view->setHasLoginFailed($isLoginFormValid ? true : false);
 $this->view->setFormValueOf('csrfToken', $this->antiCsrfService->getToken());
 $this->view->setFormValueOf('email', $this->httpRequest->findParameter('email'));
 $this->view->setFormValueOf('remember', $this->httpRequest->findParameter('remember', false));
 foreach ($this->formValidationService->findErrorsOf('Login') as $key => $value) {
 $this->view->setFormErrorCodeOf($key, $value);
 }
 $this->view->index();
 return null;
 }
 // Set persistent user login cookie if User opted to stay logged in.
 if ($this->httpRequest->findParameter('remember')) {
 $this->view->setPersistentUserLoginCookie(
 (string) $user->getIdentifier(),
 (string) $user->getAuth()->getSeriesNumber(),
 (string) $user->getAuth()->getToken()
 );
 }
 $this->view->setIsUserLoggedIn(true);
 // Token has been succesfully used.
 $this->antiCsrfService->regenerateToken();
 } else {
 $this->view->setIsFormTokenValid(false);
 }
 $this->view->index();
 }
}

You can view the LoginView class that's being controlled by this controller here, if interested.

Kid Diamond
  • 2.6k
  • 17
  • 35
lang-php

AltStyle によって変換されたページ (->オリジナル) /