php-conventions icon indicating copy to clipboard operation
php-conventions copied to clipboard

1.3. Выравнивание присвоений переменных и элементов массива

Open peter-gribanov opened this issue 5 years ago • 5 comments

1.3. Выравнивание присвоений переменных и элементов массива

Довольно спорное решение. Я бы сказал вкусовщина. Я когда-то очень давно применял этот подход и результат мне не понравился. Улучшается читаемость только если название переменных (ключей массива) и присваиваемые им значения имеют примерно одинаковую длину.

$inside_left_floating_width   = 0;
$inside_right_floating_width  = 0;
$outside_left_floating_width  = 0;
$outside_right_floating_width = 0;

Если разница в длине значительная, то образуются большие дырки которые осложняют чтение кода как в вашем примере:

$varName                            = 'varName';
$secondVariableWithVeryLongNameHere =
    '123456790123456790123456790123456790123456790123456790123456790123456790123456790';

Другой утрированный пример более наглядно демонстрирующий проблему.

$user_access_token    = $user_access_token_storage->get();
$i                    = 1;
$has_promote_articles = false;
$result               = [];
$ids                  = [];
$categories           = [];
foreach ($article_rep->slice($start, self::PER_PAGE) as $article) {
   // ...

Ещё добавление пустых отступов увеличивает длину кода объявления переменной и мы раньше упремся в лимит в 120 символов на строку и нам раньше потребуется разбивать код на строки, что приведет к увеличению длинны метода и ухудшению читаемости из-за распухших методов и мы раньше упремся в ограничение длинны метода.

$choice_translation_domain = 'article';
$total                     = $article_rep->countOf(
    new PublishedArticles(new \DateTimeImmutable()),
    new Cache(self::CACHE_TTL)
);

Вот еще кусок кода взятый из реального не моего проекта отформатированный в соответствии с вашим требованием. Отступы в начале строки нужны потому, что код имеет глубокую вложенность.

                            $descendant_delimeter = strrpos($query, '::');
                            $isChild              = substr($query, $descendant_delimeter - 5, 5) === 'child';
                            $el                   = substr($query, $descendant_delimeter + 2);
                            $query                = substr($query, 0, strrpos($query, '/')).
                                ($isChild ? '/' : '//').$el;
                            // потребовалось сделать перенос так как уперлись в 120 символов

                            $p   = $i + 1;
                            $nth = trim(mb_substr($selector, $p, strpos($selector, ')', $i) - $p));
                            // другой логический блок переменных который имеет свои отступы и
                            // вступает в диссонанс с предыдущим блоком из-за разницы длинны в отступах

Так диссонанс будет нагляднее

$descendant_delimeter = strrpos($query, '::');
$isChild              = substr($query, $descendant_delimeter - 5, 5) === 'child';
$el                   = substr($query, $descendant_delimeter + 2);
$query                = substr($query, 0, strrpos($query, '/')).($isChild ? '/' : '//').$el;

$p   = $i + 1;
$nth = trim(mb_substr($selector, $p, strpos($selector, ')', $i) - $p));

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

Я не помню, что бы мне на GitHub попадался хотя бы один проект который бы использовал такое форматирование, что как бы тоже наводит на мысль о несостоятельности решения.

peter-gribanov avatar Dec 19 '19 09:12 peter-gribanov

Спасибо за комментарий, я все больше склоняюсь к тому, что конкретно этот пункт вовсе стоит удалить из рекомендаций. Этот пункт слишком спорен и субъективен. Ваш же пример в двух видах оформления, мне субъективно читать проще с отступами. Физически проще за счет того, что есть 2 колонки, в зависимости от того, что мне надо (переменная/значение) - я читаю правую, или левую колонку. В случае, когда отступов нет - визуально нужно разделять каждую строку. Объективный минус отступов в том, что в коммит попадут помимо изменяемых строк еще и отформатированные, это усложняет code review.

Возможно вы сталкивались с плагинами для phpStorm, которые могут визуально добавлять отступы, при этом не менять исходный код?

$user_access_token    = $user_access_token_storage->get();
$i                    = 1;
$has_promote_articles = false;
$result               = [];
$ids                  = [];
$categories           = [];

$user_access_token = $user_access_token_storage->get();
$i = 1;
$has_promote_articles = false;
$result = [];
$ids = [];
$categories = [];

index0h avatar Dec 19 '19 10:12 index0h

Я не автор, но попробую поспорить)

$query                = substr($query, 0, strrpos($query, '/')).
                                ($isChild ? '/' : '//').$el;
                            // потребовалось сделать перенос так как уперлись в 120 символов

Тут плохое оформление - дело десятое, тут код: substr($query, 0, strrpos($query, '/')). ($isChild ? '/' : '//').$el - нечитаемый абсолютно. Отсюда и в 120 упёрлись

А вот тут - частично согласен. Не надо принудительно выравнивать сильно разные строки. Лучше наоборот разделить их пустой строкой, чтобы не слипалось визуально

// плохо
$varName                            = 'varName';
$secondVariableWithVeryLongNameHere =
    '123456790123456790123456790123456790123456790123456790123456790123456790123456790';

// лучше
$varName = 'varName';

$secondVariableWithVeryLongNameHere = '123456790123456790123456790123456790123456790123456790123456790123456790123456790';

VladReshet avatar Dec 19 '19 11:12 VladReshet

В принципе можно и дополнить этот пункт комментарием @VladReshet , получится что-то типа такого

$user_access_token    = $user_access_token_storage->get();
$has_promote_articles = false;

$result     = [];
$categories = [];

$i   = 1;
$ids = [];

или такого

$user_access_token    = $user_access_token_storage->get();
$has_promote_articles = false;

$i          = 1;
$result     = [];
$ids        = [];
$categories = [];

index0h avatar Dec 19 '19 11:12 index0h

Физически проще за счет того, что есть 2 колонки, в зависимости от того, что мне надо (переменная/значение) - я читаю правую, или левую колонку.

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

image

Аналогичная проблема есть и в классических таблицах и есть вполне распространенные решения этой проблемы.

image image image image

peter-gribanov avatar Dec 19 '19 11:12 peter-gribanov

@peter-gribanov убедили

index0h avatar Dec 19 '19 12:12 index0h