parent
a03c8eee49
commit
9c57f53d9e
276
%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
Normal file
276
%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
Normal file
@ -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
|
||||||
|
|
Loading…
Reference in New Issue
Block a user