diff --git a/%D0%A7%D1%82%D0%BE-%D1%82%D0%B0%D0%BA%D0%BE%D0%B5-%D1%85%D0%BE%D1%80%D0%BE%D1%88%D0%B8%D0%B9-%D0%BA%D0%BE%D0%B4.md b/%D0%A7%D1%82%D0%BE-%D1%82%D0%B0%D0%BA%D0%BE%D0%B5-%D1%85%D0%BE%D1%80%D0%BE%D1%88%D0%B8%D0%B9-%D0%BA%D0%BE%D0%B4.md new file mode 100644 index 0000000..590cbca --- /dev/null +++ b/%D0%A7%D1%82%D0%BE-%D1%82%D0%B0%D0%BA%D0%BE%D0%B5-%D1%85%D0%BE%D1%80%D0%BE%D1%88%D0%B8%D0%B9-%D0%BA%D0%BE%D0%B4.md @@ -0,0 +1,276 @@ +# Что такое хороший код + +Самое важное при написании кода — помнить, что он пишется **для людей**. Это значит, что он должен быть максимально понятным и читабельным, требовать минимум времени на понимание и на внесение изменений. Давай разберем, что именно надо делать для этого: + +### Читабельные имена + +Имена переменных, функций, классов должны быть максимально понятными. В идеале не глядя на код функции или содержимое переменной, должно быть очевидно что она делает или хранит. + +### Отвечай за слова + +Функция должна делать ровно то, что написано в ее названии. Переменная или поле должны хранить ровно то, что написано в названии. Это позволяет читать код, не тратя время на изучение кода функции или поиск, что записывается в переменную. + +### Не называй одну и ту же вещь по-разному + +Если переменная и поле объекта хранят одно и то же, они должны называться одинаково: + +```php +$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` с помощью знака вопроса, а также позволяет определить тип возвращаемого функцией значения: + +```php +/** Вычисляет рейтинг поста на PHP7 */ +function getPostRating(\DateTime $added, int $commentCount, int $likeCount): float { ... } +``` + +В будущем возможности типизации планируется расширять - возможно, будут добавлены тайп-хинты для полей классов. Если ты пока используешь PHP5, то указать тип полей, аргументов и возвращаемого значения можно с помощью phpDoc (но это лишь комментарии, проверяться они не будут): + +```php +/** +* Вычисляет рейтинг поста (в PHP5) +* +* @param \DateTime $added Дата добавления поста +* @param int $commentCount Число комментариев +* @param int $likeCount Число лайков +* @return float Значение рейтинга +*/ +function getPostRating(\DateTime $added, $commentCount, $likeCount) { ... } +``` + +К сожалению, в случае массивов пока нельзя задать структуру массива - например, нельзя указать, что это массив, содержащий объекты определенного класса. + +### Переворачивай большие if + +Бывает, что в функции почти весь код заключен в огромный if: + +```php +// Пример плохого кода +function doSomething($x, $y) +{ + if ($x > 0) { + if ($y > 0) { + // много строк + } else { + return 0; + } + } else { + return 0; + } +} +``` + +В такой ситуации лучше "перевернуть" этот `if`, поменяв в нем условие на противоположное. Это уменьшит глубину отступов и сделает структуру кода последовательной и более простой: + +```php +function doSomething($x, $y) +{ + if ($x <= 0) { + return 0; + } + + if ($y <= 0) { + return 0; + } + + // много строк +} +``` + +### Не ставь много отступов + +Если код содержит слишком много вложенных операторов вроде `if` или `for` (которые добавляют отступы), его тяжело читать, и надо переделать. Почти всегда хватает 3-4 уровней вложенности. Например, можно вынести часть кода в отдельные функции. + +### Не скрывай ошибки + +Если в твоем приложении что-то идет не так, например, отстутствует какой-то файл, который должен присуствовать, или переменная имеет неправильное значение, то не надо молчать или возвращать `false`, который никто потом не проверит. Нужно выбрасывать исключение и сообщать о том, что приложение сломано. Чем раньше ты обнаружишь ошибку, тем быстрее ее исправишь. + +Например, у нас есть функция загрузки данных из конфига: + +```php +// Пример плохого кода +function loadConfig($path) +{ + if (file_exists($path)) { + // прочитать конфиг и вернуть массив значений + } +} +``` + +Если файл конфига отсутствует, то функция просто вернет `null` и возможно, программа упадет где-то в другом месте, а возможно продолжит работать, но выдаст неправильные данные. Вместо этого нужно сразу же сообщить о проблеме, это позволит сразу ее заметить и исправить: + +```php +function loadConfig($path) +{ + if (!file_exists($path)) { + throw new ConfigLoadException("Config not found: $path"); + } + + ... +``` + +### Избегай побочных эффектов + +Хорошо написанная функция получает все нужные ей данные из аргументов (метод может еще использовать данные из полей объекта). Побочные эффекты - это случаи, когда функция или метод ведут себя не очевидно. Например: + +- функция использует глобальную переменную. Если не поместить в эту переменную заранее нужное значение, функция сработает неправильно и вернет неверный результат. +- функция или метод работает правильно только если перед ней был вызван какой-то другой метод. + +Такие вещи невозможно угадать, глядя на заголовок функции, потому можно вызвать ее не так, как задумано, и получить ошибку или неверный результат. Такого быть не должно. Функция должна всегда возвращать актуальный и точный результат, либо выбросить исключение если это сделать невозможно. Не должно быть ситуаций, когда функция возвращает неправильный результат. + +Примеры плохого кода. Глядя на заголовок, можно подумать, что +результат выполнения функции зависит только от данных пользователя и числа `$points`. +Но на самом деле он зависит от данных, указанных в `$_GET`. Если же там нет +нужных данных, данные не будут сохранены в базу: + +```php +// Пример плохого кода +/** + * Добавляет указанное число очков на счет пользователя + */ +function giveBonusPoints(User $user, $points) +{ + $currentPoints = getUserPointsFromDb($_GET['id']); + $currentPoints += $points; + + // сохраняет число очков в базе + saveUserPointsInDb($_GET['id'], $currentPoints); + $user->setPoints($currentPoints); +} +``` + +Второй пример. Функция `getBonusPoints()`, судя по названию, возвращает число бонусных очков. Однако если не вызвать функцию `init()` перед ней, она вернет ноль. Это неправильно и ведет к ошибкам. В данном случае надо либо добавить вызов функции `init()` в конструктор (чтобы она гарантированно вызывалась при создании объекта), либо избавиться от свойства `$bonusPoints` и вычислять значение прямо в `getBonusPoints()`. + +```php +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 файлов: + +```php +// неправильно +if (md5($content1) == md5($content2)) { +``` + +В данном случае быстрее сравнить содержимое напрямую чем вычислять криптографический хеш. Применение `md5()` не дает никакой выгоды: + +```php +if ($content1 == $content2) { +``` + +Также, использование `md5` для генерации имен файлов или ключей кеша часто только делает их более нечитаемыми, усложняя отладку впоследствии: + +```php +// неправильно: что мешает сразу time? +// также, это не позволит загружать более 1 файла в секунду +$filename = md5(time()) . '.jpg'; + +// плохо: нечитаемый ключ +$cacheKey = md5($cityId . $userId . $taskId); + +// хорошо: +$cacheKey = sprintf("city:%d:user:%d:task:%d", $cityId, $userId, $taskId); +``` + +### Используй константы, когда есть выбор из нескольких вариантов + +Если какой-то параметр может принимать только одно из нескольких значений, надо завести константы для их обозначения. Например, допустим у нас есть система учета сотрудников, и каждый сотрудник в ней имеет один из статусов: "находится на испытательном сроке", "работает", "уволен". В этой ситуации надо завести константы, обозначающие эти статусы. Имя константы обычно начинается с того, к какому параметру она относится (`STATUS`): + +```php +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 появилась возможность делать приватные константы - для использования только внутри класса. + +### Время не стоит на месте + +Если ты в функции получаешь текущее время, то лучше делать это один раз, и сохранять результат в переменную, так как иначе оно может измениться между вызовами. Например: + +```php +if ('2016-10-11' == date('Y-m-d') || '2016-10-10' == date('Y-m-d') { ... } +``` + +Если этот код запустить в полночь с 10 на 11 октября 2016 года, он может не сработать. Вот как надо его переписать: + +```php +$now = date('Y-m-d'); +if ('2016-10-11' == $now || '2016-10-10' == $now) { ... } +``` + +### Подписывай временные файлы + +Иногда программе для работы требуется сохранить что-то во временный файл, а потом его удалить. Иногда из-за ошибки файлы не удаляются. Чтобы было легко определить, чей это файл, стоит указать название программы в начале имени временного файла, например: + + /tmp/some-program-xtdydft5d4.tmp + \ No newline at end of file