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

[Desquire]

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

Мне был отплавен код ниже.

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

Мне отетили - это типо для второй менее важной части, должно работать на калькуляторе, за чистоту я не парился. И мол покажи мастер класс на на примере этого класса как надо...

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




PHP:



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;
}

}





Добавлю чуть инфы - я друпал дев, у нас свои кодинг стандарты (https://www.drupal.org/docs/develop/standards).

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

Ближе к делу...

Часть \ пример 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...

Ребят, пишите красивый код Старайтесь читать что-то новое,

Автору первоначального кода, я знаю ты прочитаешь - jupeter/clean-code-php (https://github.com/jupeter/clean-code-php)

@kick (https://forum.antichat.xyz/members/890001/) @Logan22 (https://forum.antichat.xyz/members/890017/) Знаю вы шарите, особенно в новых версиях пыхи, что подскажите ?)

[kick]

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

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()


Что за такой источник? Зачем передавать? Ладно бы ещё как то реквесты брало или ещё как то. Или просто по нормальному приведено, но какой от этого смысл?



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


ну тут не обязательно и использовать Guzzle, он по сути подойдет для проекта где используется с не 1 апи и не только. Но можно и curl, для быстрых и простых задач подойдёт. А так да стоит в серьезном приложение использовать его и без него никуда. На дворе почти зарелизенный php 7.4, начиная с 5.4 появились нормальные массивы вида [], любой современный фреймворк работает уже на минимум 7.2 так 7.1 в декабре закончится поддержка, а мы досих пор используем фразу array().


public $gzc = false;//сжатие


А трудно назвать gzip? где в слове gzip есть gzc? в чём проблема назвать?

Да тут много чего и что необходимо рефакторить и переписывать.

Никакого согласования в именнование переменных нет. А это опять таки нарушение PSR стандартов. Тут недавно была тема очень интересная от @Solution (https://forum.antichat.xyz/members/894690/).

https://mmo-dev.info/threads/Нужен-совет-бывалых-php.11363/ (https://forum.antichat.xyz/threads/774363/)И там были весьма полезные сообщения с полезными ссылками, продублирую их просто тут:



GitHub - piotrplenik/clean-code-php: :bathtub: Clean Code concepts adapted for PHP (https://github.com/jupeter/clean-code-php)

:bathtub: Clean Code concepts adapted for PHP. Contribute to piotrplenik/clean-code-php development by creating an account on GitHub.

github.com


GitHub - alexeymezenin/laravel-best-practices: Laravel best practices (https://github.com/alexeymezenin/laravel-best-practices)

Laravel best practices. Contribute to alexeymezenin/laravel-best-practices development by creating an account on GitHub.

github.com


https://www.sonarsource.com/docs/CognitiveComplexity.pdf

PHP: Hypertext Preprocessor (https://www.php.net)

PHP is a popular general-purpose scripting language that powers everything from your blog to the most popular websites in the world.

www.php.net




PHP Standards Recommendations - PHP-FIG (https://www.php-fig.org/psr/)

We're a group of established PHP projects whose goal is to talk about commonalities between our projects and find ways we can work better together.

www.php-fig.org


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

[mr.s4z]

Оффтоп - в друпале пишут комменты на русском?))

Я не много не понял, это твой код или клиента?)

[Desquire]

Оффтоп - в друпале пишут комменты на русском?))


Для любителей читать через строку -

Первый код НЕ МОЙ.

И как видно не связан с друпалом.



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


Надеюсь это срказм.

Так как оба ответа в первом посте.

[kick]

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


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

[Desquire]

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


Ну если человек знает что это, то скорее всего и код кидел.

Так что его не должно удивлять.

А так по части урла понять что это?

Мне например показалась бы что это апишка на AWS которая просто собирает данные.

[Низ]

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

[Desquire]

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


От куда вы взяли что клиент ?)

@I'm StreL (https://forum.antichat.xyz/members/890539/) читаю через строку спросил про клиента...

И уже он клиент

[Низ]

@Desquire (https://forum.antichat.xyz/members/890224/), не много недопоняли ... видимо я плохо вывожу свои мысли в свет. Используя, "Клиент" я имел ввиду нарицательное прилагательное в случае другого кодера.

[Logan22]

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

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

Люди одичали.

[Psycho]

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

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


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

[ade0t]

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

[Logan22]

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


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

[Desquire]

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


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

[ade0t]

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

[Mex-Vision]

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

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

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

php_shit.rar (https://drive.google.com/open?id=10qiYEB3StUoPYvVSOPcvGGv3iMpU9IIP)

drive.google.com