Чучуть шикарного кода. Порефактрим вместе?

Desquire · 28 Окт 2019

На прошлой неделе был мини диалог с один из уважаемых людей, который так или иначе связан с пхп.
Мне был отплавен код ниже.
Так как у меня чсв не маленькое, я и ответил - о это полный ппц, тут рефакторить почти все... и в твоих интересах этот код больше не кому не показывать.
Мне отетили - это типо для второй менее важной части, должно работать на калькуляторе, за чистоту я не парился. И мол покажи мастер класс на на примере этого класса как надо...

Ну а что, я простой друпал девелепер с хорошим чсв... Х*ли нет, когда да?

PHP:

<?php
/**
* Created by PhpStorm.
* User: SomeNickName
* Date: 29.08.2019
* Time: 9:15
*/

class API_MMO
{
    public $project_id = 25;
    public $act;
    public $method = 'Linage2AccountsAuth';
    public $data;
    public $gzc = false;//сжатие
    public $secret_key = 'sdfsdaf4w3r34drfcde';
    public $key = '^28Yi7j+DzQeb';
    public $session_id = false;
    public $sid = 251;
    public $api_url = 'http://youApi.s0.someurl.ru/api';


    public function __construct($session_id = false, $sid = false, $pid = false)
    {
        if ($session_id !== false)
            $this->session_id = $session_id;
        if ($sid !== false)
            $this->sid = $sid;
        if ($pid !== false)
            $this->project_id = $pid;
    }

    public function set_session($session_id){

        $this->session_id = $session_id;

    }

    /*
     *  Метод авторизации
     *  $email - Принемает емаил
     *  $password - Принемает пароль
     *  $remember - Запомнить пользователя, сессия продливается на 3 месяца
     *  $utm_source - источник входа
     *  $id_promo - ид промо бонуса
     *  $promo_bonus - бонусы выбранные пользователем
     *
     */

    public function login($email, $password, $remember = 'on', $utm_source = 'APPLICATION', $id_promo = 0, $promo_bonus = array()){
        $this->act = 'login';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'login-email' => $email,
            'login-password' => $password,
            'utm_source' => $utm_source,
            'promo' => $id_promo,
            'promo_bonus' => $promo_bonus,
            'login-remember-me' => $remember,
        );

        return $this;
    }


    public function forgot($email){
        $this->act = 'forgot';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'reminder-email' => $email,
        );

        return $this;
    }

    public function subscription($email){
        $this->act = 'subscription';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'subscription-email' => $email,
        );

        return $this;
    }

    public function register($prefix, $login, $email, $password, $password_verify, $referal = '', $utm_source = 'APPLICATION', $id_promo = 0, $promo_bonus = array()){
        $this->act = 'register';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'register-account' => $prefix . $login,
            'register-email' => $email,
            'register-password' => $password,
            'register-password-verify' => $password_verify,
            'register-referal' => $referal,
            'utm_source' => $utm_source,
            'promo' => $id_promo,
            'promo_bonus' => $promo_bonus,
        );

        return $this;
    }

    public function create_game_account($prefix, $login, $password, $password_verify,  $utm_source = 'APPLICATION'){
        $this->act = 'create_game_account';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'register-account' => $prefix . $login,
            'register-password' => $password,
            'register-password-verify' => $password_verify,
            'utm_source' => $utm_source,
        );

        return $this;
    }

    public function change_password_game_account($login, $password_old, $password_new,  $password_verify, $pin){
        $this->act = 'change_password_game_account';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'change-password-account' => $login,
            'change-password-old' => $password_old,
            'change-password-new' => $password_new,
            'change-password-verify' => $password_verify,
            'change-password-pin' => $pin,
        );

        return $this;
    }

    public function game_account_teleport_char($login, $char_id){
        $this->act = 'game_account_teleport_char';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'teleport-account' => $login,
            'teleport-char' => $char_id,

        );

        return $this;
    }


    public function recovery_password_game_account($login, $pin){

        $this->act = 'recovery_password_game_account';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'recovery-password-account' => $login,
            'recovery-password-pin' => $pin,

        );

        return $this;
    }


    public function change_password_ma($password_new, $password_old, $password_old_verify,  $pin){
        $this->act = 'recovery_pin';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'change-password-old' => $password_new,
            'change-password-new' => $password_old,
            'change-password-verify' => $password_old_verify,
            'change-password-pin' => $pin,
        );

        return $this;
    }


    public function recovery_pin(){
        $this->act = 'recovery_pin';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'action' => 'recovery-pin',
        );

        return $this;
    }

    public function disabled_hwid($login,  $pin){
        $this->act = 'disabled_hwid';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'disabled-hwid-account' => $login,
            'disabled-hwid-pin' => $pin,
        );

        return $this;
    }

    public function pin_enable(){
        $this->act = 'pin_systems';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'action' => 'pin_enable',
        );

        return $this;
    }

    public function pin_disabled($pin){
        $this->act = 'pin_systems';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'action' => 'pin_disabled',
            'change-disabled-pin' => $pin,
        );

        return $this;
    }

    public function balance_update(){
        $this->act = 'balance_update';
        $this->method = 'PaymentSystem';
        $this->data = array(
            'action' => 'balance_update',
        );

        return $this;
    }

    public function poll_msg($notice_id, $rating){
        $this->act = 'poll_msg';
        $this->method = 'NoticeSystem';
        $this->data = array(
            'notice_id' => $notice_id,
            'rating' => $rating,
        );

        return $this;
    }

    public function close_msg($notice_id){
        $this->act = 'close_msg';
        $this->method = 'NoticeSystem';
        $this->data = array(
            'notice_id' => $notice_id,
        );

        return $this;
    }

    public function select_language($lang = 'rus'){
        $this->act = 'select_language';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'lang' => $lang

        );

        return $this;
    }

    public function select_server($server_id){
        $this->act = 'select_server';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'server' => $server_id
        );

        return $this;
    }

    //Примечание если бонус код выдается на персонажа, в окне вода выводим строчки с выбором персонажа, ответ будет таким$reply['action'] == 'show_account'
    public function bonus_code($code, $account_id = 0, $char_id = 0){
        $this->act = 'activation';
        $this->method = 'BonusCod';
        $this->data = array(
            'bonus-cod' => $code,
            'account' => $account_id,
            'char' => $char_id,
        );

        return $this;
    }

    public function get_rating(){
        $this->act = 'getRating';
        $this->method = 'GameRating';
        $this->data = array(
            'temp' => 0
        );

        return $this;
    }

    public function send(){

        $Params = array(
            'act' => $this->act,
            'method' => $this->method,
            'pid' => $this->project_id,
            'data' => $this->data,
            'time' => time(),
            'gzc' => $this->gzc,
            'ip' => '127.0.0.1', //ип пользователя
            'secret_key' => $this->secret_key,
            'sid' => $this->sid,
            'user' => array(
                'name' => 'updater',
                'platform' => 'desktop',
            ),
        );

        if ($this->session_id !== false)
            $Params['session'] = $this->session_id;


        # Формируем параметры и создаем секретный ключ
        $Params = array_merge(
            array(
                "api_key" => hash("sha512", json_encode($Params) . "|" . $this->key)
            ),
            $Params
        );

        $Params = array("POST" => json_encode($Params));


        $ch = curl_init();
        curl_setopt($ch, CURLOPT_URL, $this->api_url);
        curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
        curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($Params));
        $response = curl_exec($ch);


        if($this->gzc)
            $POST = json_decode(@gzuncompress($response) ,true);
        else
            $POST = json_decode($response,true);

        return $POST;
    }

}

Добавлю чуть инфы - я друпал дев, у нас свои

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

.
Поэтому это все что дальше будет в теме, мое личое виденье и на правильность и адекватность не претендует.

Ближе к делу...
Часть \ пример 1:
У нас есть работающие приложение, в котором нужно привести в орядок ТОЛЬКО этот класс.
Логику мы НЕ меняем, только косметика.

Коментарии отсутсвуют, если мы ее знаем все приложение, то работать будет очень тяжело.
Пример пары методов с коментариями.

PHP:

/**
* Set session Id.
*
* @param string $session_id
*   Session Id.
*/
public function set_session($session_id){
  $this->session_id = $session_id;
}


/**
* Send request to our API.
*
* @return mixed
*  Decoded response.
*/
public function send() {
  $params = [
    'act' => $this->act,
    'method' => $this->method,
    'pid' => $this->project_id,
    'data' => $this->data,
    'time' => time(),
    'gzc' => $this->gzc,
    'ip' => '127.0.0.1',
    'secret_key' => $this->secret_key,
    'sid' => $this->sid,
    'user' => [
      'name' => 'updater',
      'platform' => 'desktop',
    ],
  ];

  if ($this->session_id !== FALSE) {
    $params['session'] = $this->session_id;
  }
  $params = array_merge([
    "api_key" => hash("sha512", json_encode($params) . "|" . $this->key),
  ],
    $params);

  $params = ["POST" => json_encode($params)];


  $ch = curl_init();
  curl_setopt($ch, CURLOPT_URL, $this->api_url);
  curl_setopt($ch, CURLOPT_RETURNTRANSFER, TRUE);
  curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($params));
  $response = curl_exec($ch);


  if ($this->gzc) {
    return json_decode(@gzuncompress($response), TRUE);
  }
  else {
    return json_decode($response, TRUE);
  }
}

Часть 2: wtf is going on?

2.1
Что это за значения?
Под каждое новое приложение будет меняется код?

Код:

    public $project_id = 25;
    public $method = 'Linage2AccountsAuth';
    public $secret_key = 'sdfsdaf4w3r34drfcde';
    public $key = '^28Yi7j+DzQeb';
    public $sid = 251;
    public $api_url = 'http://youApi.s0.someurl.ru/api';

Магические значения вообще вообще не ок.
Опять без пояснений кзначенияем.

2.2 Тут похоже классы ради видимости, и чтобы как-то сгруперовать функции. Так как единственная связть между ними это прокидываение екоьоры значенией.
Все.
В другой часте прилоеднеия испольузется аля

$api = new API_MMO(глобальные параметры)
$api->login(Куча параметром)
$result = $api->send();

2.3 Судя по всему, о композере и речи не идет...

Эх... зачем использовать отличные либы, когда можносделать костыли самому?
Зачем юзать Guzzle ... Есть же curl_init();

2.4
Шикарное кол-во аргументов.
У нас лимит 10. тут 9. Вроде влезле... Но...Может всетаки можно обойтись всего одним ?)

register($prefix, $login, $email, $password, $password_verify, $referal = '', $utm_source = 'APPLICATION', $id_promo = 0, $promo_bonus = array())

2.5 Хотел что-то написать про псар, но change_password_game_account и API_MMO

3 Тут должен быть вариант полного рефакторинга, и показать что все это з 300+ строк кода привратиться в 200, из которых 50 будуи коментраии...
Но я ленивая жопа, и друпал разраб, код писать не для меня.

In the end...
Ребят, пишите красивый код

Старайтесь читать что-то новое,
Автору первоначального кода, я знаю ты прочитаешь -

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

kick Logan22 Знаю вы шарите, особенно в новых версиях пыхи, что подскажите ?)

Andragor · 28 Окт 2019

комментарии с ошибками меня больше позабавили, ибо говнокод кое-как, но работает, а ошибки в коммах - вещь забавная :loltt0:

А у меня вопрос, это что?
'ip' => '127.0.0.1', //ип пользователя
Точнее, за что? Зачем? Почему так?

kick · 28 Окт 2019

Вообще начнём с самой магии)

PHP:

public $project_id = 25;
public $session_id = false;
public $sid = 251;

и теперь самое прикольное

PHP:

public function __construct($session_id = false, $sid = false, $pid = false)
    {
        if ($session_id !== false)
            $this->session_id = $session_id;
        if ($sid !== false)
            $this->sid = $sid;
        if ($pid !== false)
            $this->project_id = $pid;
    }

Оригинальное решение) Каким это магическим образом у нас типы данных вида integer, ведут в boolean? Я конечно понимаю, что пэхапе не сильно типизированный язык. Но это бред и ещё так сравнивать, когда тождественно не равно.
Далее самое интересное, а автор знает про правило, что большое количество аргументов является плохим тоном в любом языке?
Но мы идём дальше и смотрим на методы. Разве трудно ловить реквесты, и фильтровать что в них мы получили? И всё. А теперь самое интересное весь этот набор функций можно просто выкинуть. Это тупо копипаст кода и нарушение принципа DRY. За этот принцип в серьезных компаниях просто не принимают и с тобой попрощаются.
Я не буду копипастить весь код, но приведу простой пример на этом коде:

PHP:

public function subscription($email){
        $this->act = 'subscription';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'subscription-email' => $email,
        );

        return $this;
    }

    public function register($prefix, $login, $email, $password, $password_verify, $referal = '', $utm_source = 'APPLICATION', $id_promo = 0, $promo_bonus = array()){
        $this->act = 'register';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'register-account' => $prefix . $login,
            'register-email' => $email,
            'register-password' => $password,
            'register-password-verify' => $password_verify,
            'register-referal' => $referal,
            'utm_source' => $utm_source,
            'promo' => $id_promo,
            'promo_bonus' => $promo_bonus,
        );

        return $this;
    }

    public function create_game_account($prefix, $login, $password, $password_verify,  $utm_source = 'APPLICATION'){
        $this->act = 'create_game_account';
        $this->method = 'Linage2AccountsAuth';
        $this->data = array(
            'register-account' => $prefix . $login,
            'register-password' => $password,
            'register-password-verify' => $password_verify,
            'utm_source' => $utm_source,
        );

        return $this;
    }

всегда, у нас идёт одно и тоже и что за магическая переменная act? Почему не назвать для всех подробно и нормально action? Но ладно, я приведу простой пример как это можно всё уменьшить

PHP:

/**
     * @param $action
     * @param $method
     * @param array $data
     * @return $this
     */
    public function sendAction($action, $method, $data = [])
    {
        $this->act = $action;
        $this->method = $method;
        $this->data = $data;
        return $this;
    }

Всё то же самое, но всё решилось 5 строчками кода лишь передавая необходимые данные и их разбирая. У нас идёт каждый раз один и тот же код, только меняется action и method. Какой смысл и практичное применение? Не какого. Какой тут, мы передали необходимые данные и массив и всё готово без кучи магических переменных, без кучи лишнего и подобия вида:
$utm_source = 'APPLICATION', $id_promo = 0, $promo_bonus = array()
Что за такой источник? Зачем передавать? Ладно бы ещё как то реквесты брало или ещё как то. Или просто по нормальному приведено, но какой от этого смысл?

Desquire написал(а):
Эх... зачем использовать отличные либы, когда можносделать костыли самому?
Зачем юзать Guzzle ... Есть же curl_init();

ну тут не обязательно и использовать Guzzle, он по сути подойдет для проекта где используется с не 1 апи и не только. Но можно и curl, для быстрых и простых задач подойдёт. А так да стоит в серьезном приложение использовать его и без него никуда. На дворе почти зарелизенный php 7.4, начиная с 5.4 появились нормальные массивы вида [], любой современный фреймворк работает уже на минимум 7.2 так 7.1 в декабре закончится поддержка, а мы досих пор используем фразу array().
public $gzc = false;//сжатие
А трудно назвать gzip? где в слове gzip есть gzc? в чём проблема назвать?
Да тут много чего и что необходимо рефакторить и переписывать.
Никакого согласования в именнование переменных нет. А это опять таки нарушение PSR стандартов. Тут недавно была тема очень интересная от Solution.

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

И там были весьма полезные сообщения с полезными ссылками, продублирую их просто тут:

kick написал(а):
Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

А вот и наши любимые стандарты для PHP

mr.s4z · 28 Окт 2019

Оффтоп - в друпале пишут комменты на русском?))
Я не много не понял, это твой код или клиента?)

Desquire · 28 Окт 2019

I'm StreL написал(а):
Оффтоп - в друпале пишут комменты на русском?))

Для любителей читать через строку -
Первый код НЕ МОЙ.
И как видно не связан с друпалом.

I'm StreL написал(а):
Код не плох, вопрос, почему используешь curl для отправки post?)

Надеюсь это срказм.
Так как оба ответа в первом посте.

kick · 28 Окт 2019

I'm StreL написал(а):
Оффтоп - в друпале пишут комменты на русском?))
Я не много не понял, это твой код или клиента?)

клиента видно же, что сохранено

Andragor · 28 Окт 2019

Desquire написал(а):
PHP:

<?php public $api_url = 'http://youApi.s0.someurl.ru/api'; 'platform' => 'desktop', "api_key" => hash("sha512", json_encode($Params) . "|" . $this->key) }

Совсем не спалил клиента, прям загадка, кто же это :loltt0:

Desquire · 28 Окт 2019

Andragor написал(а):
Совсем не спалил клиента, прям загадка, кто же это

Ну если человек знает что это, то скорее всего и код кидел.
Так что его не должно удивлять.
А так по части урла понять что это?
Мне например показалась бы что это апишка на AWS которая просто собирает данные.

Andragor · 28 Окт 2019

Desquire написал(а):
Ну если человек знает что это, то скорее всего и код кидел.
Так что его не должно удивлять.
А так по части урла понять что это?
Мне например показалась бы что это апишка на AWS которая просто собирает данные.

там не только урл палит что за клиент

Низ · 28 Окт 2019

Мда... грусть. Сравнивая свое говно даже с кодом вашего клиента я почувствовал себя ущербным. Норм

Desquire · 28 Окт 2019

Низ написал(а):
Мда... грусть. Сравнивая свое говно даже с кодом вашего клиента я почувствовал себя ущербным. Норм

От куда вы взяли что клиент ?)
mr.s4z читаю через строку спросил про клиента...
И уже он клиент

Andragor · 28 Окт 2019

Desquire написал(а):
На прошлой неделе был мини диалог с один из уважаемых людей, который так или иначе связан с пхп.

Ну как бэ за чашкой чая или за посиделками в телеге никто не обсуждает чистоту кода просто так)

Низ · 28 Окт 2019

Desquire, не много недопоняли ... видимо я плохо вывожу свои мысли в свет. Используя, "Клиент" я имел ввиду нарицательное прилагательное в случае другого кодера.

Logan22 · 28 Окт 2019

После просмотра конструктора , дальше не смотрел ?.

Я пару дней назад видел движ джамлы под сайт ла2, на рабочий проекте. На Европе вроде.
Люди одичали.

Psycho · 28 Окт 2019

Logan22 написал(а):
После просмотра конструктора , дальше не смотрел ?.

Я пару дней назад видел движ джамлы под сайт ла2, на рабочий проекте. На Европе вроде.
Люди одичали.

Ну хоть не на юкозе)

ade0t · 28 Окт 2019

скажу одно на ГО такого не было бы

Logan22 · 28 Окт 2019

Psycho написал(а):
Ну хоть не на юкозе)

А ведь были раньше сервера на юкозе, аж ностальгия))

Desquire · 28 Окт 2019

Logan22 написал(а):
А ведь были раньше сервера на юкозе, аж ностальгия))

Там даже были готовые шаблоны под л2 тематику)

ade0t · 28 Окт 2019

и статус сервера отображало с онлайном хотя на укозе не было цуля

Mex-Vision · 28 Окт 2019

А можно скинуть кусок своего говнокода? =) Я тоже практикуюсь, в основном по видеоурокам, хотел бы узнать мнение бывалых) Автору темы могу кинуть в скайпе)

Уже один из местных кодеров посоветовл использовать синглтон, думаю могу еще узнать много интересного. Готов слушать всю критику и развиваться в этом направлении.

Вот 2 класса промо страницы с регистрацией. Контроллер и модель.

Вы не можете просматривать ссылку пожалуйста воспользуйтесь следующими ссылками Вход или Регистрация

Чучуть шикарного кода. Порефактрим вместе?

Гений мысли

Предприниматель, директор, бизнесмен

Предвестник

/* s4z */

Гений мысли

Предвестник

Предприниматель, директор, бизнесмен

Гений мысли

Предприниматель, директор, бизнесмен

Айвазовский

Гений мысли

Предприниматель, директор, бизнесмен

Айвазовский

Гений мысли

Просветленный

Заблокирован

Гений мысли

Гений мысли

Заблокирован

Бессмертный

Похожие темы