За последние 24 часа нас посетили 18975 программистов и 1614 роботов. Сейчас ищут 996 программистов ...

Не перебарщиваю ли я с определением переменных?

Тема в разделе "PHP для новичков", создана пользователем Вероломство, 10 июн 2024.

  1. Вероломство

    Вероломство Активный пользователь

    С нами с:
    19 июн 2017
    Сообщения:
    626
    Симпатии:
    24
    Есть некий ГК:

    PHP:
    1. readonly class PostSigninAction
    2. {
    3.     public function __construct(
    4.         private Form         $form,
    5.         private UserRecord   $userRecord,
    6.         private AuthSession  $authSession,
    7.         private FlashSession $flashSession
    8.     )
    9.     {
    10.     }
    11.  
    12.     public function __invoke(Request $request, Response $response): Response
    13.     {
    14.         if ($this->form->button('signin')) {
    15.             $login = $this->form->query('login');
    16.             $password = $this->form->query('password');
    17.  
    18.             $user = $this->userRecord->getFromLogin($login);
    19.  
    20.             if ($user && password_verify($password, $user->getPasswordHash())) {
    21.                 $token = uniqid();
    22.  
    23.                 $user->setToken($token);
    24.                 $user->save();
    25.  
    26.                 $id = $user->getId();
    27.                 $token = $user->getToken();
    28.  
    29.                 $this->authSession->set($id, $token);
    30.  
    31.                 return $response->redirect('/menu');
    32.             }
    33.  
    34.             $this->flashSession->danger('Вы не ввели или неправильно ввели данные');
    35.         }
    36.  
    37.         return $response->redirect();
    38.     }
    39. }
    Может мне нужно попроще быть и сделать так? :)

    PHP:
    1. readonly class PostSigninAction
    2. {
    3.     public function __construct(
    4.         private Form         $form,
    5.         private UserRecord   $userRecord,
    6.         private AuthSession  $authSession,
    7.         private FlashSession $flashSession
    8.     )
    9.     {
    10.     }
    11.  
    12.     public function __invoke(Request $request, Response $response): Response
    13.     {
    14.         if ($this->form->button('signin')) {
    15.             $user = $this->userRecord->getFromLogin($this->form->query('login'));
    16.  
    17.             if ($user && password_verify($this->form->query('password'), $user->getPasswordHash())) {
    18.                 $user->setToken(uniqid());
    19.                 $user->save();
    20.              
    21.                 $this->authSession->set($user->getId(), $user->getToken());
    22.  
    23.                 return $response->redirect('/menu');
    24.             }
    25.  
    26.             $this->flashSession->danger('Вы не ввели или неправильно ввели данные');
    27.         }
    28.  
    29.         return $response->redirect();
    30.     }
    31. }
     
  2. amberson

    amberson Активный пользователь

    С нами с:
    23 июл 2020
    Сообщения:
    65
    Симпатии:
    16
    Не перебарщиваешь. Код должен читаться легко, первый вариант читается легче. Ты сказал что может быть написать попроще и сделал сложнее. Первый вариант выглядит как раз проще. Больше строк не значит сложнее. Ну на мой взгляд.
     
    Вероломство и artoodetoo нравится это.
  3. MouseZver

    MouseZver Суперстар

    С нами с:
    1 апр 2013
    Сообщения:
    7.788
    Симпатии:
    1.328
    Адрес:
    Лень
    2 вариант жизненный. В первом например ты иницилизируешь token и заносишь в переменную, последующее в set, но дальше.. какой смысл переписывать переменную токена когда она и тем же значением существовала ? Логическое лишнее действие.

    Насчет длинных строк в условиях if
    Лучше делать как в первом - читаемость. Иначе немного на вермишель в мясорубке похожа, хоть и сам порой страдаю такой хней.
     
    Вероломство нравится это.
  4. MouseZver

    MouseZver Суперстар

    С нами с:
    1 апр 2013
    Сообщения:
    7.788
    Симпатии:
    1.328
    Адрес:
    Лень
    PHP:
    1. <?php
    2.  
    3. //Controller/Auth
    4. public function login(): CommitRepository
    5. {
    6.     if ( isGranted( 'ROLE_USER' ) )
    7.     {
    8.         return $this -> redirect( '/' );
    9.     }
    10.    
    11.     $this -> setSingleTitles( 'Авторизация' );
    12.    
    13.     $login = $this -> model( \Auth\Login :: class );
    14.    
    15.     if ( isMethodPost() )
    16.     {
    17.         try
    18.         {
    19.             $user = $login -> validation( function ( UserInterface $user ) use ( $login ): bool
    20.             {
    21.                 return $this -> getPasswordHasher( $user ) -> verify( $user -> getPassword(), $login -> getRequestPassword(), $user -> getSalt() );
    22.             } );
    23.            
    24.             $login -> sendEmail( $user );
    25.            
    26.             $login -> authUser( $user );
    27.            
    28.             return $this -> redirect( '/' );
    29.         }
    30.         catch ( ViolationsException $e )
    31.         {
    32.             if ( isRequestXHR() )
    33.             {
    34.                 return $this -> customJson( RestApi :: success() -> errorMessages( ...$e -> getErrors() ) );
    35.             }
    36.            
    37.             request() -> attributes -> set( ViolationsException :: class, $e );
    38.         }
    39.     }
    40.    
    41.     return $this -> render( 'auth/login', 'auth/auth-template' );
    42. }
    43.  
    44. // Model/Auth/Login
    45.  
    46. public function authUser( UserInterface $user ): void
    47. {
    48.     container( \AuthenticationHandler :: class )
    49.         -> createSession( $user, 'secured_area', request() -> request -> has( 'remember_check' ) );
    50. }
     
    Вероломство нравится это.
  5. Вероломство

    Вероломство Активный пользователь

    С нами с:
    19 июн 2017
    Сообщения:
    626
    Симпатии:
    24
    @amberson@MouseZver принято, благодарю, порешали
     
  6. Вероломство

    Вероломство Активный пользователь

    С нами с:
    19 июн 2017
    Сообщения:
    626
    Симпатии:
    24
    @MouseZver смысл апдейта токена в том, что если залогинился, например, в другом браузере, то в первом будет разлогин, так и задумано, нельзя сидеть на сайте с нескольких устройств
     
  7. MouseZver

    MouseZver Суперстар

    С нами с:
    1 апр 2013
    Сообщения:
    7.788
    Симпатии:
    1.328
    Адрес:
    Лень
    Но это уже другая тема
     
    Вероломство нравится это.
  8. Вероломство

    Вероломство Активный пользователь

    С нами с:
    19 июн 2017
    Сообщения:
    626
    Симпатии:
    24
    @MouseZver сори я не сразу догнал: подумал ты об изменении переменной, я всё понял - переменную я определяю и потом второй раз геттером её же значение и забираю, виноват, осознал :)