Skip to content

Latest commit

 

History

History
executable file
·
276 lines (195 loc) · 18.4 KB

good-code.md

File metadata and controls

executable file
·
276 lines (195 loc) · 18.4 KB

Что такое хороший код

Самое важное при написании кода — помнить, что он пишется для людей. Это значит, что он должен быть максимально понятным и читабельным, требовать минимум времени на понимание и на внесение изменений. Давай разберем, что именно надо делать для этого:

Читабельные имена

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

Отвечай за слова

Функция должна делать ровно то, что написано в ее названии. Переменная или поле должны хранить ровно то, что написано в названии. Это позволяет читать код, не тратя время на изучение кода функции или поиск, что записывается в переменную.

Не называй одну и ту же вещь по-разному

Если переменная и поле объекта хранят одно и то же, они должны называться одинаково:

$name = $_POST['name'];
$user->name = $name;
$user->setName($name);

$userManager = new UserManager;

Допустимое исключение - когда используются разные стили написания. Например, мы можем называть переменную $userName, а поле в базе данных user_name.

Не храни 2 разных вещи в одной переменной

Неправильно, например, хранить в одной переменной $winner сначала число очков, а затем - название победившей команды. Это собьет с толку того, кто вносит изменения в код и повышает шансы, что он допустит ошибку.

С другой стороны (не множь сущности без необходимости), не надо заводить несколько разных переменных для одной и той же сущности. Если ты обрабатываешь строку с текстом, не стоит на каждую операцию заводить новую переменную, можно использовать ту же самую: $text = trim($text);

Результат функции

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

Нельзя делать так, что в каком-то случае функция может вернуть неправильное значение.

Тип должен быть один

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

Допустимо использовать вариант "тип или null", например функция может возвращать объект класса User или null, если пользователь не найден.

PHP5 позволяет с помощью тайп-хинта указать тип аргумента, если это объект, функция (callable) или массив (array), а также разрешить использование null. PHP7 расширяет возможности тайп-хинтинга и позволяет указывать любой тип, включая int или string, позволяет указать допустимость null с помощью знака вопроса, а также позволяет определить тип возвращаемого функцией значения:

/** Вычисляет рейтинг поста на PHP7 */
function getPostRating(\DateTime $added, int $commentCount, int $likeCount): float { ... }

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

/**
* Вычисляет рейтинг поста (в PHP5)
*
* @param    \DateTime   $added          Дата добавления поста
* @param    int         $commentCount   Число комментариев
* @param    int         $likeCount      Число лайков
* @return   float       Значение рейтинга
*/
function getPostRating(\DateTime $added, $commentCount, $likeCount) { ... }

К сожалению, в случае массивов пока нельзя задать структуру массива - например, нельзя указать, что это массив, содержащий объекты определенного класса.

Переворачивай большие if

Бывает, что в функции почти весь код заключен в огромный if:

// Пример плохого кода
function doSomething($x, $y) 
{
    if ($x > 0) {
        if ($y > 0) {
            // много строк
        } else {
            return 0;
        }
    } else {    
        return 0;
    }
}

В такой ситуации лучше "перевернуть" этот if, поменяв в нем условие на противоположное. Это уменьшит глубину отступов и сделает структуру кода последовательной и более простой:

function doSomething($x, $y) 
{
    if ($x <= 0) {
        return 0;
    }
    
    if ($y <= 0) {
        return 0;
    }
    
    // много строк
}

Не ставь много отступов

Если код содержит слишком много вложенных операторов вроде if или for (которые добавляют отступы), его тяжело читать, и надо переделать. Почти всегда хватает 3-4 уровней вложенности. Например, можно вынести часть кода в отдельные функции.

Не скрывай ошибки

Если в твоем приложении что-то идет не так, например, отстутствует какой-то файл, который должен присуствовать, или переменная имеет неправильное значение, то не надо молчать или возвращать false, который никто потом не проверит. Нужно выбрасывать исключение и сообщать о том, что приложение сломано. Чем раньше ты обнаружишь ошибку, тем быстрее ее исправишь.

Например, у нас есть функция загрузки данных из конфига:

// Пример плохого кода
function loadConfig($path) 
{
    if (file_exists($path)) {
        // прочитать конфиг и вернуть массив значений
    }
}

Если файл конфига отсутствует, то функция просто вернет null и возможно, программа упадет где-то в другом месте, а возможно продолжит работать, но выдаст неправильные данные. Вместо этого нужно сразу же сообщить о проблеме, это позволит сразу ее заметить и исправить:

function loadConfig($path)
{
    if (!file_exists($path)) {
        throw new ConfigLoadException("Config not found: $path");
    }
    
    ...    

Избегай побочных эффектов

Хорошо написанная функция получает все нужные ей данные из аргументов (метод может еще использовать данные из полей объекта). Побочные эффекты - это случаи, когда функция или метод ведут себя не очевидно. Например:

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

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

Примеры плохого кода. Глядя на заголовок, можно подумать, что результат выполнения функции зависит только от данных пользователя и числа $points. Но на самом деле он зависит от данных, указанных в $_GET. Если же там нет нужных данных, данные не будут сохранены в базу:

// Пример плохого кода
/**
 * Добавляет указанное число очков на счет пользователя
 */
function giveBonusPoints(User $user, $points)
{
    $currentPoints = getUserPointsFromDb($_GET['id']);
    $currentPoints += $points;

    // сохраняет число очков в базе
    saveUserPointsInDb($_GET['id'], $currentPoints);
    $user->setPoints($currentPoints);
}

Второй пример. Функция getBonusPoints(), судя по названию, возвращает число бонусных очков. Однако если не вызвать функцию init() перед ней, она вернет ноль. Это неправильно и ведет к ошибкам. В данном случае надо либо добавить вызов функции init() в конструктор (чтобы она гарантированно вызывалась при создании объекта), либо избавиться от свойства $bonusPoints и вычислять значение прямо в getBonusPoints().

class BonusCalculator
{
    private $bonusPoints = 0;

    public function init()
    {
        $this->bonusPoints = 1;

        // Нет ли тут ошибки в 2 вызовах?
        if (date('j') == '1' || date('j') == '3') {
            // В 1-й и 3-й день месяца бонус больше
            $this->bonusPoints = 2;
        }
    }

    public function getBonusPoints()
    {
        return $this->bonusPoints;
    }
}

$bonusCalculator = new BonusCalculator;
echo $bonusCalculator->getBonusPoints();

Не злоупотребляй хешированием

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

// неправильно
if (md5($content1) == md5($content2)) {

В данном случае быстрее сравнить содержимое напрямую чем вычислять криптографический хеш. Применение md5() не дает никакой выгоды:

if ($content1 == $content2) {

Также, использование md5 для генерации имен файлов или ключей кеша часто только делает их более нечитаемыми, усложняя отладку впоследствии:

// неправильно: что мешает сразу time? 
// также, это не позволит загружать более 1 файла в секунду
$filename = md5(time()) . '.jpg'; 

// плохо: нечитаемый ключ
$cacheKey = md5($cityId . $userId . $taskId);

// хорошо: 
$cacheKey = sprintf("city:%d:user:%d:task:%d", $cityId, $userId, $taskId);

Используй константы, когда есть выбор из нескольких вариантов

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

class Employee
{
    const STATUS_TRIAL = 'trial'; // На исп. сроке
    const STATUS_FULLTIME = 'fulltime'; // Работает постоянно
    const STATUS_LEFT = 'left'; // Покинул компанию
    
    public $status;
    ...
}

$employee = new Employee;
$employee->status = Employee::STATUS_FULLTIME;

Использование констант дает такие преимущества в сравнении с числами или строками:

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

Код не должен полагаться на конкретные значения констант, то есть если мы в коде выше поменяем const STATUS_FULLTIME = 'fulltime' на const STATUS_FULLTIME = 1;, всё должно работать по-прежнему.

Если варианта всего 2, и они примерно по смыслу соответствуют "да" и "нет", можно вместо констант использовать значения true/false.

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

Время не стоит на месте

Если ты в функции получаешь текущее время, то лучше делать это один раз, и сохранять результат в переменную, так как иначе оно может измениться между вызовами. Например:

if ('2016-10-11' == date('Y-m-d') || '2016-10-10' == date('Y-m-d') { ... }

Если этот код запустить в полночь с 10 на 11 октября 2016 года, он может не сработать. Вот как надо его переписать:

$now = date('Y-m-d');
if ('2016-10-11' == $now || '2016-10-10' == $now) { ... }

Подписывай временные файлы

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

/tmp/some-program-xtdydft5d4.tmp