Все дело в том, что так бывает...

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

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

Кусочек сыра

Сегодня попался на глаза вот такой кусочек кода.

<?php
//code
if (!Utilities::arrayContains($array1, $array2)) {
    return false;
}
//code
?>

Думаю что-то не так. Создают функцию в классе Utilities только тогда, когда не могут определить ее принадлежность. Что уже странно. Во-вторых, ну название. Вроде бы и понятно все, с другой стороны, не внушает какого-то доверия.

<?php
/**
 * @param array $array1
 * @param array $array2
 * @return boolean
 */
public static function arrayContains(array $array1, array $array2)
{
    foreach ($array1 as $value1) {
        if (!in_array($value1, $array2)) {
            return false;
        }
    }

    return true;
}
?>

Действительно, несколько раз пробегаю глазами, так как боюсь упустить что-то, заставляя себя поверить в увиденное. Аккуратно оставляю предложение заменить содержание на <?php empty(array_diff($array1, $array2));?>, переименовать или удалить метод.

В итоге вечером получаем...

<?php
/**
 * @param array $array1
 * @param array $array2
 * @return boolean
 */
public static function isArrayDiff(array $array1, array $array2)
{
    $diff = array_diff($array1, $array2);

    return empty($diff);
}
?>

Про лису

Хитрый лис не скажет вороне: "Дай сыр, птица". Во-первых, она может не дать его. Во-вторых, навряд ли это чему-то научит, басня-то не получится и морали не будет. Поэтому идеальный вариант для ревью - это подача чего угодно в наилучшем виде для автора кода. В более сложных случаях я и свой вариант содержания не предлагаю, так как его у меня просто нет. Просто прошу еще раз подумать, в сложных ситуациях спасает парное программирование. При этом не надо выступать в роли оратора или диктатора. Если вы хотите думать за всех, пишите проект один тогда. Не могу сказать, что у меня всегда получается быть хитропопой лисой, но я стараюсь.

Про чистый код и ворону

Я думаю, что знаю на 99 процентов причину появления такого метода. Конечно, человек уже наелся определенного куска проекта. Метод вызывается в очень неприятном месте с кучей условных конструкций, хотелось как-то лучше разгрести. Скорее всего, в голову не пришла функция array_diff, и вынос foreach в отдельный метод - правильный выбор.

Но сам урок начался позже. Методы из двух строчек имеют место быть, все дело в названии. В последнем варианте мы получили обертку над стандартной функцией. В данном контексте она бессмысленна. Если мы захотим реализовать работу со стеком в ООП стиле и обернуть array_pop и array_push в класс и методы, то у нас будет цель и причина всего этого. Здесь цель не прослеживается.

Так что же делать в данном случае? Я бы подумал над куском кода, где это вызывается. Возможно, существует определение этой операции в целом. Ведь массивы что-то содержат, у них есть нормальные имена (в статье они преднамеренно переведены в array1 и array2).

Например, если бы array1 содержал права пользователя, а array2 все необходимые права на просмотр страницы. Тогда, на мой скромный взгляд, метод имеет место быть на 101 процент, с каким-нибудь именем isAllow*.

Если мы не можем как-то определить и выделить по смыслу эти строчки, то и выносить их, скорее всего, нет смысла.

Мораль басни

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

Вы что-то прочитали, у вас куча идей и желание все перехерачить. А что-то идеальное лежит почему-то всегда на "горном перевале". Так бывает, что на эмоциях мы проскакиваем его. Прочитали о паттернах - давай плодить из plain php кучу классов, выстраивать архитектуру, которая в итоге ничуть не понятнее. Прочитали дядюшку Боба - давай все дробить без остановки. Прочитали про agile - давай ломать работу проекта. Прочитали статью - давай сразу внедрять.

Я сам постоянно допускаю подобные ошибки, но при этом думаю, что это правильно. Продолжая горную тему: это лучше, чем сидеть перед перевалом всю жизнь, не совершая каких-то дерзких попыток покорения вершин, или вообще спуститься в долину со словами: "Да ну нафиг эти горы". С приобретением опыта наши просчеты уменьшаются, а какими путниками мы станем, зависит только от нас.

Статью завершу словами: "Лучше гор могут быть только горы, на которых никто не бывал".