Это принципы разработки ПО, взятые из книги Clean Code Роберта Мартина и адаптированные для PHP. Это руководство не по стилям программирования, а по созданию читабельного, многократно используемого и пригодного для рефакторинга кода на PHP.
Не каждый из этих принципов должен строго соблюдаться, и ещё с меньшим количеством все будут согласны. Это лишь рекомендации, не более, но все они кодифицированы в многолетнем коллективном опыте автора Clean Code.
Статья вдохновлена clean-code-javascript.
Содержание
- Переменные
- Функции
- Объекты и структуры данных
- Классы
- S: Принцип единственной ответственности (Single Responsibility Principle, SRP)
- O: Принцип открытости/закрытости (Open/Closed Principle, OCP)
- L: Принцип подстановки Барбары Лисков (Liskov Substitution Principle, LSP)
- I: Принцип разделения интерфейса (Interface Segregation Principle, ISP)
- D: Принцип инверсии зависимостей (Dependency Inversion Principle, DIP)
Переменные
Используйте значимые и произносимые имена переменных
Плохо:
$ymdstr = $moment->format('y-m-d');Хорошо:
$currentDate = $moment->format('y-m-d');Для одного типа переменных используйте единый словарь
Плохо:
getUserInfo();
getClientData();
getCustomerRecord();Хорошо:
getUser();Используйте имена, по которым удобно искать
Мы прочитаем больше кода, чем когда-либо напишем. Поэтому важно писать такой код, который будет читабелен и удобен для поиска. Но давая переменным имена, бесполезные для понимания нашей программы, мы мешаем будущим читателям. Используйте такие имена, по которым удобно искать.
Плохо:
// What the heck is 86400 for?
addExpireAt(86400);Хорошо:
// Declare them as capitalized `const` globals.
interface DateGlobal {
const SECONDS_IN_A_DAY = 86400;
}
addExpireAt(DateGlobal::SECONDS_IN_A_DAY);Используйте пояснительные переменные
Плохо:
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/';
preg_match($cityZipCodeRegex, $address, $matches);
saveCityZipCode($matches[1], $matches[2]);Неплохо:
Так лучше, но мы всё ещё сильно зависим от регулярного выражения.
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/';
preg_match($cityZipCodeRegex, $address, $matches);
list(, $city, $zipCode) = $matches;
saveCityZipCode($city, $zipCode);Хорошо:
С помощью именования подпаттернов снижаем зависимость от регулярного выражения.
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(?<city>.+?)\s*(?<zipCode>\d{5})?$/';
preg_match($cityZipCodeRegex, $address, $matches);
saveCityZipCode($matches['city'], $matches['zipCode']);Избегайте ментального сопоставления
Не заставляйте тех, кто будет читать ваш код, переводить значения переменных. Лучше писать явно, чем неявно.
Плохо:
$l = ['Austin', 'New York', 'San Francisco'];
for ($i = 0; $i < count($l); $i++) {
$li = $l[$i];
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
// Wait, what is `$li` for again?
dispatch($li);
}Хорошо:
$locations = ['Austin', 'New York', 'San Francisco'];
foreach ($locations as $location) {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
dispatch($location);
});Не добавляйте ненужный контекст
Если имя вашего класса/объекта с чем-то у вас ассоциируется, не проецируйте эту ассоциацию на имя переменной.
Плохо:
$car = [
'carMake' => 'Honda',
'carModel' => 'Accord',
'carColor' => 'Blue',
];
function paintCar(&$car) {
$car['carColor'] = 'Red';
}Хорошо:
$car = [
'make' => 'Honda',
'model' => 'Accord',
'color' => 'Blue',
];
function paintCar(&$car) {
$car['color'] = 'Red';
}Вместо сокращённых или условных используйте аргументы по умолчанию
Плохо:
function createMicrobrewery($name = null) {
$breweryName = $name ?: 'Hipster Brew Co.';
// ...
}Хорошо:
function createMicrobrewery($breweryName = 'Hipster Brew Co.') {
// ...
}Функции
Аргументы функций (в идеале два или меньше)
Крайне важно ограничивать количество параметров функций, потому что это упрощает тестирование. Больше трёх аргументов ведёт к «комбинаторному взрыву», когда вам нужно протестировать кучу разных случаев применительно к каждому аргументу.
Идеальный вариант — вообще без аргументов. Один-два тоже нормально, но трёх нужно избегать. Если их получается больше, то нужно объединять, чтобы уменьшить количество. Обычно если у вас больше двух аргументов, то функция делает слишком много. В тех случаях, когда это не так, чаще всего в качестве аргумента достаточно использовать более высокоуровневый объект.
Плохо:
function createMenu($title, $body, $buttonText, $cancellable) {
// ...
}Хорошо:
class MenuConfig
{
public $title;
public $body;
public $buttonText;
public $cancelLabel = false;
}
$config = new MenuConfig();
$config->title = 'Foo';
$config->body = 'Bar';
$config->buttonText = 'Baz';
$config->cancelLabel = true;
function createMenu(MenuConfig $config) {
// ...
}Функции должны делать что-то одно
Это, безусловно, самое важное правило в разработке ПО. Когда функции делают больше одной вещи, их труднее составлять, тестировать и обосновывать. А если вы можете наделить функции только какими-то одиночными действиями, то их будет легче рефакторить, а ваш код станет гораздо чище. Даже если вы не будете следовать никакой другой рекомендации, кроме этой, то всё равно опередите многих других разработчиков.
Плохо:
function emailClients($clients) {
foreach ($clients as $client) {
$clientRecord = $db->find($client);
if ($clientRecord->isActive()) {
email($client);
}
}
}Хорошо:
function emailClients($clients) {
$activeClients = activeClients($clients);
array_walk($activeClients, 'email');
}
function activeClients($clients) {
return array_filter($clients, 'isClientActive');
}
function isClientActive($client) {
$clientRecord = $db->find($client);
return $clientRecord->isActive();
}Имена функций должны быть говорящими
Плохо:
function addToDate($date, $month) {
// ...
}
$date = new \DateTime();
// It's hard to tell from the function name what is added
addToDate($date, 1);Хорошо:
function addMonthToDate($month, $date) {
// ...
}
$date = new \DateTime();
addMonthToDate(1, $date);Функции должны быть лишь одним уровнем абстракции
Если у вас несколько уровней абстракции, то на функцию возложено слишком много задач. Разбиение функций позволяет многократно использовать код и облегчает тестирование.
Плохо:
function parseBetterJSAlternative($code) {
$regexes = [
// ...
];
$statements = split(' ', $code);
$tokens = [];
foreach($regexes as $regex) {
foreach($statements as $statement) {
// ...
}
}
$ast = [];
foreach($tokens as $token) {
// lex...
}
foreach($ast as $node) {
// parse...
}
}Хорошо:
function tokenize($code) {
$regexes = [
// ...
];
$statements = split(' ', $code);
$tokens = [];
foreach($regexes as $regex) {
foreach($statements as $statement) {
$tokens[] = /* ... */;
});
});
return $tokens;
}
function lexer($tokens) {
$ast = [];
foreach($tokens as $token) {
$ast[] = /* ... */;
});
return $ast;
}
function parseBetterJSAlternative($code) {
$tokens = tokenize($code);
$ast = lexer($tokens);
foreach($ast as $node) {
// parse...
});
}Уберите дублирующий код
Старайтесь полностью избавиться от дублирующего кода. Он плох тем, что если вам нужно менять логику, то это придётся делать в нескольких местах.
Представьте, что вы владеете ресторанчиком и отслеживаете, есть ли продукты: помидоры, лук, чеснок, специи и т. д. Если у вас несколько списков с содержимым холодильников, то вам придётся обновлять их все, когда вы готовите какое-то блюдо. А если список один, то и вносить изменения придётся только в него.
Зачастую дублирующий код возникает потому, что вы делаете две и более вещи, у которых много общего. Но небольшая разница между ними заставляет вас писать несколько функций, и те по большей части делают одно и то же. Удаление дублирующего кода означает, что вы создаёте абстракцию, которая может обрабатывать все различия с помощью единственной функции/модуля/класса.
Правильный выбор абстракции критически важен, поэтому нужно следовать принципам SOLID, описанным в разделе «Классы». Плохие абстракции могут оказаться хуже дублирующего кода, так что будьте осторожны! Но если можете написать хорошие, то делайте это! Не повторяйтесь, иначе окажется, что при каждом изменении вам нужно обновлять код в нескольких местах.
Плохо:
function showDeveloperList($developers) {
foreach($developers as $developer) {
$expectedSalary = $developer->calculateExpectedSalary();
$experience = $developer->getExperience();
$githubLink = $developer->getGithubLink();
$data = [
$expectedSalary,
$experience,
$githubLink
];
render($data);
}
}
function showManagerList($managers) {
foreach($managers as $manager) {
$expectedSalary = $manager->calculateExpectedSalary();
$experience = $manager->getExperience();
$githubLink = $manager->getGithubLink();
$data = [
$expectedSalary,
$experience,
$githubLink
];
render($data);
}
}Хорошо:
function showList($employees) {
foreach($employees as $employe) {
$expectedSalary = $employe->calculateExpectedSalary();
$experience = $employe->getExperience();
$githubLink = $employe->getGithubLink();
$data = [
$expectedSalary,
$experience,
$githubLink
];
render($data);
}
}Не используйте флаги в качестве параметров функций
Флаги говорят вашим пользователям, что функции делают больше одной вещи. А они должны делать что-то одно. Разделяйте свои функции, если они идут по разным ветвям кода в соответствии с булевой логикой.
Плохо:
function createFile($name, $temp = false) {
if ($temp) {
touch('./temp/'.$name);
} else {
touch($name);
}
}Хорошо:
function createFile($name) {
touch($name);
}
function createTempFile($name) {
touch('./temp/'.$name);
}Избегайте побочных эффектов
Функция может привносить побочные эффекты, если она не только получает значение и возвращает другое значение/значения, но и делает что-то ещё. Побочным эффектом может быть запись в файл, изменение глобальной переменной или случайная отправка всех ваших денег незнакомому человеку.
Но иногда побочные эффекты бывают нужны. Например, та же запись в файл. Лучше делать это централизованно. Не выбирайте несколько функций и классов, которые пишут в какой-то один файл, используйте для этого единый сервис. Единственный.
Главная задача — избежать распространённых ошибок вроде общего состояния для нескольких объектов без какой-либо структуры; использования изменяемых типов данных, которые могут быть чем-то перезаписаны; отсутствия централизованной обработки побочных эффектов. Если вы сможете это сделать, то будете счастливее подавляющего большинства других программистов.
Плохо:
// Global variable referenced by following function.
// If we had another function that used this name, now it'd be an array and it could break it.
$name = 'Ryan McDermott';
function splitIntoFirstAndLastName() {
global $name;
$name = preg_split('/ /', $name);
}
splitIntoFirstAndLastName();
var_dump($name); // ['Ryan', 'McDermott'];Хорошо:
$name = 'Ryan McDermott';
function splitIntoFirstAndLastName($name) {
return preg_split('/ /', $name);
}
$newName = splitIntoFirstAndLastName($name);
var_dump($name); // 'Ryan McDermott';
var_dump($newName); // ['Ryan', 'McDermott'];Не пишите в глобальные функции
Засорение глобалами — дурная привычка в любом языке, потому что вы можете конфликтовать с другой библиотекой, а пользователи вашего API не будут об этом знать, пока не получат исключение в production. Приведу пример: вам нужен конфигурационный массив. Вы пишете глобальную функцию вроде config(), но она может конфликтовать с другой библиотекой, пытающейся делать то же самое. Поэтому лучше использовать шаблон проектирования «синглтон» и простую конфигурацию.
Плохо:
function config() {
return [
'foo' => 'bar',
]
}Хорошо:
class Configuration {
private static $instance;
private function __construct() {/* */}
public static function getInstance() {
if (self::$instance === null) {
self::$instance = new Configuration();
}
return self::$instance;
}
public function get($key) {/* */}
public function getAll() {/* */}
}
$singleton = Configuration::getInstance();Инкапсулирование условных выражений
Плохо:
if ($fsm->state === 'fetching' && is_empty($listNode)) {
// ...
}Хорошо:
function shouldShowSpinner($fsm, $listNode) {
return $fsm->state === 'fetching' && is_empty($listNode);
}
if (shouldShowSpinner($fsmInstance, $listNodeInstance)) {
// ...
}Избегайте негативных условных выражений
Плохо:
function isDOMNodeNotPresent($node) {
// ...
}
if (!isDOMNodeNotPresent($node)) {
// ...
}Хорошо:
function isDOMNodePresent($node) {
// ...
}
if (isDOMNodePresent($node)) {
// ...
}Избегайте условных выражений
Наверно, это кажется невозможным. Впервые это услышав, многие говорят: «Как я смогу что-либо сделать без выражения if?» Второй распространённый вопрос: «Ну, это прекрасно, но зачем мне это?» Ответ заключается в рассмотренном выше правиле чистого кода: функция должна делать что-то одно. Если у вас есть классы и функции, содержащие выражение if, то тем самым вы говорите своим пользователям, что функция делает больше одной вещи. Не забывайте — нужно оставить что-то одно.
Плохо:
class Airplane {
// ...
public function getCruisingAltitude() {
switch ($this->type) {
case '777':
return $this->getMaxAltitude() - $this->getPassengerCount();
case 'Air Force One':
return $this->getMaxAltitude();
case 'Cessna':
return $this->getMaxAltitude() - $this->getFuelExpenditure();
}
}
}Хорошо:
class Airplane {
// ...
}
class Boeing777 extends Airplane {
// ...
public function getCruisingAltitude() {
return $this->getMaxAltitude() - $this->getPassengerCount();
}
}
class AirForceOne extends Airplane {
// ...
public function getCruisingAltitude() {
return $this->getMaxAltitude();
}
}
class Cessna extends Airplane {
// ...
public function getCruisingAltitude() {
return $this->getMaxAltitude() - $this->getFuelExpenditure();
}
}Избегайте проверки типов (часть 1)
PHP не типизирован, т. е. ваши функции могут принимать аргументы любых типов. Иногда такая свобода даже мешает и возникает соблазн выполнять проверку типов в функциях. Но есть много способов этого избежать. Первое, что нужно учитывать, это согласованные API.
Плохо:
function travelToTexas($vehicle) {
if ($vehicle instanceof Bicycle) {
$vehicle->peddle($this->currentLocation, new Location('texas'));
} else if ($vehicle instanceof Car) {
$vehicle->drive($this->currentLocation, new Location('texas'));
}
}Хорошо:
function travelToTexas($vehicle) {
$vehicle->move($this->currentLocation, new Location('texas'));
}Избегайте проверки типов (часть 2)
Если вы работаете с базовыми примитивами (вроде строковых, целочисленных) и массивами, то не можете использовать полиморфизм. Но если кажется, что вам всё ещё нужна проверка типов, то примените объявление типов или строгий режим (strict mode). Это даст вам статичную типизацию поверх стандартного PHP-синтаксиса. Проблема ручной проверки типов в том, что её качественное выполнение подразумевает такое многословие, что полученная искусственная «типобезопасность» не компенсирует потери читабельности кода. Сохраняйте чистоту своего PHP, пишите хорошие тесты и проводите качественные ревизии кода. Или делайте всё это, но со строгим объявлением PHP-типов или в строгом режиме.
Плохо:
function combine($val1, $val2) {
if (is_numeric($val1) && is_numeric($val2)) {
return $val1 + $val2;
}
throw new \Exception('Must be of type Number');
}Хорошо:
function combine(int $val1, int $val2) {
return $val1 + $val2;
}Убирайте мёртвый код
Он плох так же, как и дублирующий код. Не нужно держать его в кодовой базе. Если что-то не вызывается, избавьтесь от этого! Если что, мёртвый код можно будет достать из истории версий.
Плохо:
function oldRequestModule($url) {
// ...
}
function newRequestModule($url) {
// ...
}
$req = new newRequestModule($requestUrl);
inventoryTracker('apples', $req, 'www.inventory-awesome.io');Хорошо:
function requestModule($url) {
// ...
}
$req = new requestModule($requestUrl);
inventoryTracker('apples', $req, 'www.inventory-awesome.io');Объекты и структуры данных
Используйте геттеры и сеттеры
В PHP можно задать для методов ключевые слова public, protected и private. С их помощью вы будете управлять изменением свойств объекта.
- Если вам нужно не только получать свойство объекта, то необязательно находить и менять каждый метод чтения (accessor) в кодовой базе.
- Благодаря
setпроще добавить валидацию. - Можно инкапсулировать внутреннее представление.
- С помощью геттеров и сеттеров легко добавлять журналирование и обработку ошибок.
- При наследовании такого класса вы можете переопределить функциональность по умолчанию.
- Вы можете лениво загружать свойства объекта, например получая их с сервера.
Также это часть принципа открытости/закрытости, входящего в набор объектно ориентированных принципов проектирования.
Плохо:
class BankAccount {
public $balance = 1000;
}
$bankAccount = new BankAccount();
// Buy shoes...
$bankAccount->balance -= 100;Хорошо:
class BankAccount {
private $balance;
public function __construct($balance = 1000) {
$this->balance = $balance;
}
public function withdrawBalance($amount) {
if ($amount > $this->balance) {
throw new \Exception('Amount greater than available balance.');
}
$this->balance -= $amount;
}
public function depositBalance($amount) {
$this->balance += $amount;
}
public function getBalance() {
return $this->balance;
}
}
$bankAccount = new BankAccount();
// Buy shoes...
$bankAccount->withdrawBalance($shoesPrice);
// Get balance
$balance = $bankAccount->getBalance();У объектов должны быть личные/защищённые компоненты (members)
Плохо:
class Employee {
public $name;
public function __construct($name) {
$this->name = $name;
}
}
$employee = new Employee('John Doe');
echo 'Employee name: '.$employee->name; // Employee name: John DoeХорошо:
class Employee {
protected $name;
public function __construct($name) {
$this->name = $name;
}
public function getName() {
return $this->name;
}
}
$employee = new Employee('John Doe');
echo 'Employee name: '.$employee->getName(); // Employee name: John DoeКлассы
Принцип единственной ответственности (Single Responsibility Principle, SRP)
Как говорится в книге Clean Code: «Для изменения класса никогда не должно быть более одной причины». Порой заманчиво набить класс всевозможной функциональностью, как мы это делаем с сумками и рюкзаками, которые разрешается взять в качестве ручной клади в самолёт. Проблема в том, что ваш класс не будет концептуально связанным (conceptually cohesive), и поэтому возникнет много причин изменить его. Важно минимизировать количество случаев, когда вам нужно изменять класс. А важно потому, что когда в классе избыток функциональности и вам нужно поменять её часть, то может быть трудно понять, как это отразится на зависимых модулях в кодовой базе.
Плохо:
class UserSettings {
private $user;
public function __construct($user) {
$this->user = user;
}
public function changeSettings($settings) {
if ($this->verifyCredentials()) {
// ...
}
}
private function verifyCredentials() {
// ...
}
}Хорошо:
class UserAuth {
private $user;
public function __construct($user) {
$this->user = user;
}
protected function verifyCredentials() {
// ...
}
}
class UserSettings {
private $user;
public function __construct($user) {
$this->user = $user;
$this->auth = new UserAuth($user);
}
public function changeSettings($settings) {
if ($this->auth->verifyCredentials()) {
// ...
}
}
}Принцип открытости/закрытости (Open/Closed Principle, OCP)
Как сказал Бертран Мейер: «Программные сущности (классы, модули, функции и т. д.) должны быть открыты для расширения, но закрыты для модифицирования». Что это означает? Позвольте пользователям добавлять новую функциональность без изменения кода.
Плохо:
abstract class Adapter {
protected $name;
public function getName() {
return $this->name;
}
}
class AjaxAdapter extends Adapter {
public function __construct() {
parent::__construct();
$this->name = 'ajaxAdapter';
}
}
class NodeAdapter extends Adapter {
public function __construct() {
parent::__construct();
$this->name = 'nodeAdapter';
}
}
class HttpRequester {
private $adapter;
public function __construct($adapter) {
$this->adapter = $adapter;
}
public function fetch($url) {
$adapterName = $this->adapter->getName();
if ($adapterName === 'ajaxAdapter') {
return $this->makeAjaxCall($url);
} else if ($adapterName === 'httpNodeAdapter') {
return $this->makeHttpCall($url);
}
}
protected function makeAjaxCall($url) {
// request and return promise
}
protected function makeHttpCall($url) {
// request and return promise
}
}Хорошо:
abstract class Adapter {
abstract protected function getName();
abstract public function request($url);
}
class AjaxAdapter extends Adapter {
protected function getName() {
return 'ajaxAdapter';
}
public function request($url) {
// request and return promise
}
}
class NodeAdapter extends Adapter {
protected function getName() {
return 'nodeAdapter';
}
public function request($url) {
// request and return promise
}
}
class HttpRequester {
private $adapter;
public function __construct(Adapter $adapter) {
$this->adapter = $adapter;
}
public function fetch($url) {
return $this->adapter->request($url);
}
}Принцип подстановки Барбары Лисков (Liskov Substitution Principle, LSP)
За этим пугающим термином скрывается очень простая идея. Формальное определение: «Если S — это подтип Т, то объекты типа Т могут быть заменены объектами типа S (например, вместо объектов типа Т можно подставить объекты типа S) без изменения каких-либо свойств программы (корректность, задачи и т. д.)». Ещё более пугающее определение.
Можно объяснить проще: если у вас есть родительский и дочерний классы, тогда они могут быть взаимозаменяемы без получения некорректных результатов. Рассмотрим классический пример с квадратом и прямоугольником. С точки зрения математики квадрат — это прямоугольник, но если смоделировать эту взаимосвязь is-a посредством наследования, то у вас будут проблемы.
Плохо:
class Rectangle {
private $width, $height;
public function __construct() {
$this->width = 0;
$this->height = 0;
}
public function setColor($color) {
// ...
}
public function render($area) {
// ...
}
public function setWidth($width) {
$this->width = $width;
}
public function setHeight($height) {
$this->height = $height;
}
public function getArea() {
return $this->width * $this->height;
}
}
class Square extends Rectangle {
public function setWidth($width) {
$this->width = $this->height = $width;
}
public function setHeight(height) {
$this->width = $this->height = $height;
}
}
function renderLargeRectangles($rectangles) {
foreach($rectangle in $rectangles) {
$rectangle->setWidth(4);
$rectangle->setHeight(5);
$area = $rectangle->getArea(); // Плохо: Will return 25 for Square. Should be 20.
$rectangle->render($area);
});
}
$rectangles = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles($rectangles);Хорошо:
abstract class Shape {
private $width, $height;
abstract public function getArea();
public function setColor($color) {
// ...
}
public function render($area) {
// ...
}
}
class Rectangle extends Shape {
public function __construct {
parent::__construct();
$this->width = 0;
$this->height = 0;
}
public function setWidth($width) {
$this->width = $width;
}
public function setHeight($height) {
$this->height = $height;
}
public function getArea() {
return $this->width * $this->height;
}
}
class Square extends Shape {
public function __construct {
parent::__construct();
$this->length = 0;
}
public function setLength($length) {
$this->length = $length;
}
public function getArea() {
return $this->length * $this->length;
}
}
function renderLargeRectangles($rectangles) {
foreach($rectangle in $rectangles) {
if ($rectangle instanceof Square) {
$rectangle->setLength(5);
} else if ($rectangle instanceof Rectangle) {
$rectangle->setWidth(4);
$rectangle->setHeight(5);
}
$area = $rectangle->getArea();
$rectangle->render($area);
});
}
$shapes = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles($shapes);Принцип разделения интерфейса (Interface Segregation Principle, ISP)
Согласно ISP, «Клиенты не должны зависеть от интерфейсов, которые не используют».
Хороший пример демонстрации принципа: классы, для которых требуются большие объекты настроек (settings objects). Рекомендуется не требовать от клиентов настраивать много параметров, потому что по большей части они им не нужны. Если сделать их опциональными, то это поможет избежать раздутости интерфейса.
Плохо:
interface WorkerInterface {
public function work();
public function eat();
}
class Worker implements WorkerInterface {
public function work() {
// ....working
}
public function eat() {
// ...... eating in launch break
}
}
class SuperWorker implements WorkerInterface {
public function work() {
//.... working much more
}
public function eat() {
//.... eating in launch break
}
}
class Manager {
/** @var WorkerInterface $worker **/
private $worker;
public function setWorker(WorkerInterface $worker) {
$this->worker = $worker;
}
public function manage() {
$this->worker->work();
}
}Хорошо:
interface WorkerInterface extends FeedableInterface, WorkableInterface {
}
interface WorkableInterface {
public function work();
}
interface FeedableInterface {
public function eat();
}
class Worker implements WorkableInterface, FeedableInterface {
public function work() {
// ....working
}
public function eat() {
//.... eating in launch break
}
}
class Robot implements WorkableInterface {
public function work() {
// ....working
}
}
class SuperWorker implements WorkerInterface {
public function work() {
//.... working much more
}
public function eat() {
//.... eating in launch break
}
}
class Manager {
/** @var $worker WorkableInterface **/
private $worker;
public function setWorker(WorkableInterface $w) {
$this->worker = $w;
}
public function manage() {
$this->worker->work();
}
}Принцип инверсии зависимостей (Dependency Inversion Principle, DIP)
Этот принцип гласит:
- Высокоуровневые модули не должны зависеть от низкоуровневых. Оба вида должны зависеть от абстракций.
- Абстракции не должны зависеть от деталей. Детали должны зависеть от абстракций.
Сначала это может быть трудным для понимания, но если вы работали с PHP-фреймворками (вроде Symfony), то уже встречались с реализацией этого принципа в виде инъекции зависимости (Dependency Injection, DI). Однако эти принципы не идентичны, DI ограждает высокоуровневые модули от деталей своих низкоуровневых модулей и их настройки. Это может быть сделано посредством DI. Огромное преимущество в том, что снижается сцепление (coupling) между модулями. Сцепление — очень плохой шаблон разработки, затрудняющий рефакторинг кода.
Плохо:
class Worker {
public function work() {
// ....working
}
}
class Manager {
/** @var Worker $worker **/
private $worker;
public function __construct(Worker $worker) {
$this->worker = $worker;
}
public function manage() {
$this->worker->work();
}
}
class SuperWorker extends Worker {
public function work() {
//.... working much more
}
}Хорошо:
interface WorkerInterface {
public function work();
}
class Worker implements WorkerInterface {
public function work() {
// ....working
}
}
class SuperWorker implements WorkerInterface {
public function work() {
//.... working much more
}
}
class Manager {
/** @var Worker $worker **/
private $worker;
public function __construct(WorkerInterface $worker) {
$this->worker = $worker;
}
public function manage() {
$this->worker->work();
}
}Объединяйте методы в цепочки
Это очень полезный и распространённый шаблон, используемый во многих библиотеках, например в PHPUnit и Doctrine. Он делает вашу кодовую базу более выразительной и менее многословной. Поэтому я рекомендую объединять методы в цепочки (chaining), и вы сами увидите, насколько чистым станет ваш код. В конце каждой функции класса просто возвращайте this — и сможете прикреплять к нему следующий метод класса.
Плохо:
class Car {
private $make, $model, $color;
public function __construct() {
$this->make = 'Honda';
$this->model = 'Accord';
$this->color = 'white';
}
public function setMake($make) {
$this->make = $make;
}
public function setModel($model) {
$this->model = $model;
}
public function setColor($color) {
$this->color = $color;
}
public function dump() {
var_dump($this->make, $this->model, $this->color);
}
}
$car = new Car();
$car->setColor('pink');
$car->setMake('Ford');
$car->setModel('F-150');
$car->dump();Хорошо:
class Car {
private $make, $model, $color;
public function __construct() {
$this->make = 'Honda';
$this->model = 'Accord';
$this->color = 'white';
}
public function setMake($make) {
$this->make = $make;
// NOTE: Returning this for chaining
return $this;
}
public function setModel($model) {
$this->model = $model;
// NOTE: Returning this for chaining
return $this;
}
public function setColor($color) {
$this->color = $color;
// NOTE: Returning this for chaining
return $this;
}
public function dump() {
var_dump($this->make, $this->model, $this->color);
}
}
$car = (new Car())
->setColor('pink')
->setMake('Ford')
->setModel('F-150')
->dump();Композиция лучше наследования
Как говорится в известной книге «Шаблоны проектирования» Банды четырёх, по мере возможности нужно выбирать композицию, а не наследование. Есть много хороших причин использовать как наследование, так и композицию. Главная цель этой максимы заключается в том, если вы инстинктивно склоняетесь к наследованию, то постарайтесь представить, может ли композиция лучше решить вашу задачу. В каких-то случаях это действительно более подходящий вариант.
Вы спросите: «А когда лучше выбирать наследование?» Всё зависит от конкретной задачи, но можно ориентироваться на этот список ситуаций, когда наследование предпочтительнее композиции:
- Ваше наследование — это взаимосвязь is-a, а не has-a. Пример: Человек > Животное vs. Пользователь > Детали пользователя (UserDetails).
- Вы можете повторно использовать код из базовых классов. (Люди могут двигаться, как животные.)
- Вы хотите внести глобальные изменения в производные классы, изменив базовый класс. (Изменение расхода калорий у животных во время движения.)
Плохо:
class Employee {
private $name, $email;
public function __construct($name, $email) {
$this->name = $name;
$this->email = $email;
}
// ...
}
// Bad because Employees "have" tax data.
// EmployeeTaxData is not a type of Employee
class EmployeeTaxData extends Employee {
private $ssn, $salary;
public function __construct($ssn, $salary) {
parent::__construct();
$this->ssn = $ssn;
$this->salary = $salary;
}
// ...
}Хорошо:
class EmployeeTaxData {
private $ssn, $salary;
public function __construct($ssn, $salary) {
$this->ssn = $ssn;
$this->salary = $salary;
}
// ...
}
class Employee {
private $name, $email, $taxData;
public function __construct($name, $email) {
$this->name = $name;
$this->email = $email;
}
public function setTaxData($ssn, $salary) {
$this->taxData = new EmployeeTaxData($ssn, $salary);
}
// ...
}Комментарии (113)
shornikov
01.09.2017 16:16+1foreach ($locations as $location)
Сам так делаю, но начинаю сомневаться в правильности такого подхода. Переменные получаются излишне схожими (хоть за пределами цикла вторая обычно больше не используется), да и в скорости набора такой вариант проигрывает (я про то, что приличные ide подставляют по первым набранным буквам: тут приходится выбирать из двух вариантов).Eldhenn
01.09.2017 16:37-3Я обычно пишу
foreach ($locations as $loc)
Внутри блока обычно понятно, что такое $loc (или $p[erson], или $s[tudent]).
zenn
01.09.2017 16:55Аналогично, очень часто, делая нечто подобное:
$records = User::all(); foreach ($records as $record) { // some actions with $record->objects }
автокомплит, к примеру у phpstorm внутри перебора foreach(){} сует именно records вместо ожидаемой record. Отсюда у меня так же имеется ряд сомнений относительно данной рекомендации (или адекватности автокомплита phpstorm).
mdErrDX5341
01.09.2017 17:18я обычно пишу так
$userList = User::all(); foreach ($userList as $user){ //code }
Но я бы не советовал так делать… так как вроде не стандарт, но визуально так проще и пропадает проблема с news...
dmz9
02.09.2017 12:55+1на такой автокомплит грех жаловаться:
$mice = $children = $matches = $photos = $wives = $loaves = $women = $geese = $data = []; foreach ( $mice as $mouse ) {} foreach ( $children as $child ) {} foreach ( $matches as $match ) {} foreach ( $photos as $photo ) {} foreach ( $wives as $wife ) {} foreach ( $loaves as $loaf ) {} foreach ( $women as $woman ) {} foreach ( $geese as $goose ) {} foreach ( $data as $datum ) {}
про data=>datum вовсе не знал пока впервые в самом же шторме не увидел, оказалось это совершенно корректно
technokid
02.09.2017 12:56У вас
User::all()может вернуть пустой массив или обьект. И у вас нету проверки, а сразу идет циклzenn
02.09.2017 13:56Вообще, код — чисто символичный и ничего он там не может вернуть другого, кроме коллекции объектов (см. laravel doc). И да, давно уже никто не проверяет результаты выборок (по крайней мере в разумных orm) на false, null или прочую дрянь, т.к. методы возвращают ожидаемые результаты одного типа (кроме ситуативных, типа ->first() или ->firstOrNull()).
mdErrDX5341
02.09.2017 15:31интерпретатор не будет жаловаться… но как и написали этот код чисто символичный
$newsList = $newsRepository->all() foreach($newsList as $news) { //code } //$news = $newsList->first(); //$news = $newsList->last();
меня только одно интересует, многих раздражает слово list в конце, просто я постоянно использую такую практику а feedback пока не почувствовал, но опять повторюсь, не используйте...
n0dwis
02.09.2017 20:15Я использую приставку cur, и, возможно, сокращаю имя переменной. Т.е.
foreach ($locations as $curLocation)
или
foreach ($locations as $curLoc)
Второй вариант выглядит не очень, но отлично срабатывает автокомплит, да и проблем с чтением кода не возникает.
win0err
01.09.2017 16:22Тут чушь написана. Такое просто не захватит переменную $name.
`
$name = 'Ryan McDermott';
function splitIntoFirstAndLastName() {
$name = preg_split('/ /', $name);
}
splitIntoFirstAndLastName();
var_dump($name); // ['Ryan', 'McDermott'];
// Не ['Ryan', 'McDermott'], а string(14) 'Ryan McDermott'
`
Хотя вот так прокатит:
win0err
01.09.2017 16:33Прошу прощения, нечаянно отправил комментарий, не могу изменить его уже.
Хотя вот так прокатит:
$name = 'Ryan McDermott';
$splitIntoFirstAndLastName = function () use (&$name) {
$name = preg_split('/ /', $name);
};
$splitIntoFirstAndLastName();
var_dump($name); // ['Ryan', 'McDermott'];
Но такой код ещё нужно додуматься написать
nokimaro
01.09.2017 17:32-3или внутри функции надо было объявить
global $name;
function splitIntoFirstAndLastName() { global $name; $name = preg_split('/ /', $name); }
SerafimArts
01.09.2017 19:24+3За глобалсы надо отрубать всё, что выпирает. Подумайте над этим на досуге =))))
ghost404
05.09.2017 16:50В этом и суть. По моему где-то даже высказывались о бессмысленности этого правила
Dragomeat
01.09.2017 17:51-4Только что проверил код с глобалом, всё работает как и у автора статьи. Вывод: вы немного поспешили со своим комментарием)
win0err
01.09.2017 18:39+1Внутри функции другая область видимости.
Можно захватить переменную извне тремя способами, в статье не используется ни один из них:
1. Лямбда с use, как описал выше;
2. Вариант nokimaro с global внутри функции;
3. Использовать $GLOBALS, но это дичь какая-то:
<?php $name = 'Ryan McDermott'; function splitIntoFirstAndLastName() { $GLOBALS['name'] = preg_split('/ /', $GLOBALS['name']); } splitIntoFirstAndLastName(); var_dump($name); // ['Ryan', 'McDermott'];
Чтобы не соврать, проверил все варианты (и вариант из статьи) в PHP 5.6 и 7.0, а также прочёл документацию, вдруг забыл что.
php.net/manual/en/language.variables.scope.php
porutchik
01.09.2017 21:39+3Так и не понял, что за 86400. Можно же проще написать 24*60*60 и сразу всё понятно.
zoonman
02.09.2017 05:52-11Внезапно ваш код оказывается на Марсе и надо использовать местные сутки.
24*60*60 уже не прокатит.
daggert
02.09.2017 08:37+8Вы наверное не понимаете сути такой записи: 86400 — сложно прочесть и на вскидку сказать сутки это или нет, тогда как записав форматом часы*минуты*секунды — получается более гибко для изменений и более читабельно.
isden
02.09.2017 17:40А еще можно делать вот так:
strtotime('1 day', 0)
daggert
02.09.2017 20:42ИМХО конечно, но зачем городить лишние функции? Планета вроде не собирается менять орбиту, скорость вращения и наклон, то есть мы всегда знаем что в сутках 24 часа, в часе 60 минут, в минуте 60 секунд, а это означает, как минимум, что созданная давныыым давно запись часы*минуты*секунды и понятна и легко записывается одним простым int
zoonman
04.09.2017 01:01Лично мне легко это прочесть и сказать, сутки это или нет. Увы, такие базовые вещи, как количество секунд сутках можно и знать.
Как было сказано в статье, вынесение этих чисел в константы с корректным именем решает проблему восприятия.
Не нужно потом запоминать, что там.
Могу привести немного высосанный из пальца пример.
В PHP есть встроенная константа ?, но для наглядности допустим, что ее нет.
Если вы в коде будете повсюду писать что-то вроде 3.14, то вначале все будет хорошо.
Но если внезапно потребуется увеличить точность вычислений, то прийдется прошерстить весь код заменив 3.14 на 3.14159.
Если еще раз понадобится это сделать, то еще прийдется менять весь код.
Если вы где-то в одном месте недопишете одну цифру, например 3.1415, то автозамена уже не сработает. Это место будет приводить к неправильным результатам.
Если бы всюду использовали нечто Math::PI, то проблема была бы решена раз и навсегда.daggert
04.09.2017 09:34> Как было сказано в статье, вынесение этих чисел в константы с корректным именем решает проблему восприятия.
Я думаю что с этим утверждением спорить глупо (:
Szer
02.09.2017 10:44+5Если Вы реально на такие случаи закладываетесь в своём коде, мне жалко Вашего работодателя/клиентов, потому что Вы делаете какую-то херню.
michael_vostrikov
02.09.2017 15:41+1Внезапно, 86400 секунд на Марсе это все равно 24*60*60 секунд.
zoonman
04.09.2017 01:12-1Внезапно в марсианских сутках 88775 земных секунд.
michael_vostrikov
04.09.2017 05:17+2Ну и что? Замена 86400 на 24*60*60 от этого не становится неправильной.
porutchik
02.09.2017 16:13На Марсе в сутках 86400 марсианских секунд.
mdErrDX5341
03.09.2017 12:30-1Смысл в том, что магические символы это всегда дурной тон в программировании...
zoonman
04.09.2017 01:14-1Написанный код не станет марсианским, также как и системные часы не станут марсианскими. Они продолжат отсчитывать время в земных секундах.
Поэтому проще обновить прошивку ПО, понимающую, что в марсианских сутках 88775 земных секунд.
alex6636
02.09.2017 12:56Лол, ну попробуйте такое записать в свойство класса по умолчанию и посмотрите что скажет интерпретатор
sumanai
02.09.2017 14:23+2Ваши знания сильно устарели. С 5.6 в константы можно пихать вычисляемые выражения.
alex6636
02.09.2017 15:03-2оно то хорошо, хотя мы пока пишем 5.4 совместимый код. кстати, причем здесь константы?
sumanai
02.09.2017 15:08+2хотя мы пока пишем 5.4 совместимый код
Подкиньте начальству идею, что PHP 7 работает в 2 раза быстрее.
кстати, причем здесь константы?
В примере ХОРОШО используются константы класса, так как число секунд в сутках вещь константная, в большинстве случаев.alex6636
02.09.2017 15:11-3это пока не так актуально, во-первый нет пока адекватных сборок для линукса с php 7, во-вторых, заметного прироста производительности не будет, т.к основные тормоза связаны с базой данных. поэтому уж точно нет смысла переписывать старый код, другое дело, что может есть смысл писать новый 100% совместимым под 7.0.
я просто комментарий писал к свойству класса, а не константе, но да, в данном случае правильнее использовать константу конечноsumanai
02.09.2017 15:20+3во-первый нет пока адекватных сборок для линукса с php 7,
Как там, в 2015?
во-вторых, заметного прироста производительности не будет, т.к основные тормоза связаны с базой данных
Мне кажется, из-за того, что все пишут про тормоза БД, все забили на код, и как раз он сейчас является причиной тормозов. Например на типичном форуме на phpBB версий 3.1 и новее обработка кода занимает в 20 раз больше времени, чем работа с БД.
поэтому уж точно нет смысла переписывать старый код
А там и не нужно переписывать, если кон конечно не писался во времена PHP 3. Поправить нужно пару мест типа preg_replace с модификатором e, но этот код уже давно попахивал и сыпал варнингами.
Sannis
02.09.2017 15:33Когда я увидел в коде коллеги const list = array(1, 2, 3); мне поплохело. Если так можно написать не значит что нужно.
ghost404
05.09.2017 16:56Мне и моим голлегам проще читать цифры вида 3600 и 86400, хотя авторы со мной не согласны
EverOne
01.09.2017 21:54Заголовок спойлераclass BankAccount { ... public function withdrawBalance($amount) { if ($amount > $this->balance) { throw new \Exception('Amount greater than available balance.'); } $this->balance -= $amount; } ... }win0err
01.09.2017 22:19+1Лучше делать так, чтобы потом было легко отловить исключение:
<?php class BalanceException extends \Exception { } // Ну или BankAccountException, зависит уже от конкретной ситуации class BankAccount { ... public function withdrawBalance($amount) { if ($amount > $this->balance) { throw new BalanceException('Amount greater than available balance.'); } $this->balance -= $amount; } ... }EverOne
02.09.2017 00:08+1Я не об отлове (хотя и о нем тоже), а вообще об исключениях в бизнес-логике. Исключения должны обрабатывать ошибки в работе системы (такие как невозможность чтения из БД или неправильный тип аргумента функции), а не ошибки в действиях пользователя (такие как неверный пароль или недостаточная сумма на балансе, например). Я не ошибаюсь?
SerafimArts
02.09.2017 01:08В целом, нет, не ошибаешься, но исключения, вида NotFoundHttpException, UnprocessableEntityHttpException и прочие — говорят об обратном.
dmz9
02.09.2017 13:39+3Исключения должны обрабатывать ошибки в работе системы
не так.
Исключения нужны для передачи ошибок вверх по стеку вызовов
+ в них можно указать экстра-данные (если как «белый человек» расширяешь стандартный класс, добавляешь методы для get/set этих данных)
+ они избавляют все промежуточные классы/методы между throw и catch от необходимости
а. знать о кодах и типах ошибок «нижестоящих» классов
б. знать о том как их обработать
в. иметь лишнюю ответственность в виде «я могу/должен это обработать»
г. (как следствие) иметь лишний защитный код
д. иметь много типов возврата из метода (return false для ошибки, string для текста ошибки, string[] для набора ошибок)
Исключение прерывает поток исполнения, а обычная ошибка, которая отдается в return — это продолжение потока исполнения, как будто бы так и надо. Поэтому исключения нельзя использовать для нормального ветвления логики и следует использовать если из за конкретной ошибки нет возможности работать дальше (в текущем контексте, а не во всей программе). Типичный пример — «первая линия обороны» — проверка аргументов в методах где нет доверия входным данным.
Ошибки надо возвращать когда их ожидают.
Ошибки валидатора — ожидаемые. Собери их в массив для начала, засунь в исключение и брось.
Сверху, тот кто вызвал валидатор, поймает исключение и получит нужные данные для формирования юзер-френдли ответа (и логирования и еще чего нибудь).
Если брать конкретно этот простой пример — выброс исключения оправдан.
Можно ли продолжить операцию при отрицательном балансе? — нет, нельзя (хотя это зависит от бизнеса), поэтому тут именно исключение.
Намного проще поставить один try/catch где то наверху для критического кода и ожидать в нем исключения, нежели распихивать обработку в каждый класс, да еще и передачу по стеку вызов данных определенного типа и рассказывать каждому классу как работать с ними.
VolCh
02.09.2017 21:11+2Недостаточная сумма на балансе — не ошибка пользователя в методе, изменяющем баланс. Ошибки пользовательского ввода должны проверяться раньше, чем попадут в бизнес-логику, например на фронте или в контроллере, а уж если попали в модель, то попытка овердрафта — это исключение, возможно требующее вмешательства СБ. Надо понимать разницу между правилом валидации «сумма операции не должна быть меньше баланса» и бизнес-правило «ни при каких операциях итоговый баланс не должен быть меньше нуля», даже если проверка осуществляется идентичным или даже одним и тем же кодом.
kloppspb
02.09.2017 01:21Не добавляйте ненужный контекст
О, да! Это (как и многое другое) не только к PHP относится, конечно же.
Сейчас работаю с базой, проектировщик которой за каким-то лешим решил все айдишники во всех таблицах обозвать «имя_таблицы_id». Учитывая, что таблиц в схеме около сотни, и встречаются среди них с именами вроде «some_cool_stuff_report_queue», хочется убивать…JSmitty
02.09.2017 22:23селекты упрощаются немного, т.к. если есть табличка rooms и в ней PK room_id, то все ссылки на нее из других таблиц называются room_id, и тогда например можно писать такое:
SELECT * FROM persons LEFT JOIN rooms USING (room_id)
Если колонка PK называется всегда id, то потребуется полностью писать — ON (rooms.id = persons.room_id)kloppspb
03.09.2017 06:11В 50k строк исходников, работающих с этой базой, я не обнаружил ни одного подобного джойна.
michael_vostrikov
03.09.2017 10:53+1Сомнительное преимущество, писать везде лишний префикс, чтобы сэкономить пару символов в нескольких джойнах, которые во многих случаях делаются ORM.
gmini
06.09.2017 09:57Это вопрос религиозный — есть оракловский подход, называть свой ключ ID, а внешние ключи another_table_id.
и есть MS SQL подход — во всех таблицах называть ключи <table_name>_id.
Как указали ниже, это связано с синтаксисом джойнов в разных диалектах SQL
vtvz_ru
02.09.2017 01:49Аргументы по умолчанию не работают, если по умолчанию переменная ($this->default или self::$default), хотя это бывает необходимо
zoonman
02.09.2017 06:37+2Принцип подстановки Барбары Лисков продемонстрирован достаточно сложно для понимания.
Дело в том, что существуют высота и ширина в квадрате равные между собой. Поэтому установка любого из этих свойств является корректной операцией.
$area = $rectangle->getArea(); // Плохо: Will return 25 for Square. Should be 20.
Самый безграмотный комментарий, который я встречал. It must be 25 for Square.
Дело в том, что квадрат это частный случай прямоугольника, у которого стороны равны. Спешу заметить, что квадрат это еще и ромб, и параллелограмм.
Осознание что прямоугольник может быть квадратом позволяет предположить, что последовательность установки разных высот и ширин будет влиять на значение площади. Поэтому следует ожидать специфического поведения для частных случаев.
Принцип LSP сводится к избеганию когнитивной нагрузки от этих частных случаев путем их явного декларирования. Пример показывает, что вместо наследования от прямоугольника, все же лучше использовать два разных класса с общим предком в виде фигуры.
Лично мне больше понравилось описание на Stackoverflow.
В двух словах я бы описал его так: когда у вас есть два похожих, но совершенно разных объекта, не наследуйте один от другого.
michael_vostrikov
02.09.2017 16:21-1Поэтому установка любого из этих свойств является корректной операцией.
Изменение ширины у математического квадрата или прямоугольника это некорректная операция. У квадрата нет изменяющихся сторон. Если сторона изменилась, значит мы задали другой квадрат. А раз другой, то он вполне может быть прямоугольником независимо от исходного квадрата.
Если квадрат меняет стороны, значит у нас какой-то свой объект, который мы почему-то называем квадратом.
Проблема в том, что в современном ООП нет возможности описать связь, что квадрат это прямоугольник. Вот если бы тип менялся при изменении стороны, проблемы бы не было.
Может я не прав, но в примере из статьи я не вижу большой проблемы. Площадь квадрата будет нормально считаться после изменения сторон. Это не будет работать только в каких-нибудь тестах, где проверяется конкретный результат. Но в тестах лучше проверять потомков, а не родителя. Проблема будет, если будет меняться поведение или добавляться ограничения, например, setWidth() будет все под-объекты уничтожать, к которым есть доступ снаружи.
daggert
02.09.2017 22:02Проблема в том, что в современном ООП нет возможности описать связь, что квадрат это прямоугольник.
class Квадрат extends Прямоугольник?
sumanai
02.09.2017 23:27Там в следующем предложении описание ошибочности этого подхода в данном случае.
daggert
03.09.2017 13:33Честно говоря так и не понимаю ошибочности данного подхода. Наверное я слишком узко смотрю?
michael_vostrikov
03.09.2017 17:09+1Если коротко, квадрат добавляет ограничения в реализацию предка-прямоугольника, что запрещено LSP.
michael_vostrikov
03.09.2017 11:02Считается, что это нарушает SOLID. Тут есть обсуждение по этой теме.
zoonman
04.09.2017 01:47Проблема в том, что в современном ООП нет возможности описать связь, что квадрат это прямоугольник. Вот если бы тип менялся при изменении стороны, проблемы бы не было.
Именно поэтому и производится разделение на разные классы.
Чисто из геометрического смысла, попытка задать высоту или ширину у квадрата приводит к автоматическому изменению ширины или высоты соответственно.
Просто как ни крути, но прямоугольник и квадрат хоть и одного поля, но разные ягоды.michael_vostrikov
04.09.2017 05:20Разные-то разные, но связь между ними есть. А ООП не позволяет ее выразить. Про квадрат и прямоугольник знают все, а вот с какими-нибудь бизнес-сущностями это уже не так просто.
jtiq
02.09.2017 12:56+1Я тоже придерживаюсь правил, но когда устаю, то уже у меня получается как попало :( почти как быдло-кодинг
Как с этим бороться?
F0iL
02.09.2017 14:04Обязательным прохождением code-review от коллег, которые будут не давать мержиться и больно бить по рукам за любой дурнопахнущий код?)
zoonman
04.09.2017 01:52При этом нужно ментально перестроиться к тому, что к тебе не придираются, а наоборот помогают.
Многие новички воспринимают Code Review очень болезненно, как страшный суд. Поэтому задача старших коллег быть как можно осторожнее в выражениях и стараться объяснять, почему именно код плох и как его можно улучшить.
den_rad
03.09.2017 01:19Я не пушу вечером. Лучше утром свежим взглядом просмотреть код и сделать push.
И code review рулит, конечно
Travelcamnet
02.09.2017 12:56-4$locations = ['Austin', 'New York', 'San Francisco'];
ПЛОХО
foreach ($locations as $location) { doStuff(); doSomeOtherStuff(); dispatch($location); });
В каждом цикле PHP будет пересоздавать переменную. Так же данный код работает заметно медленней, чем следующие два.
ПЛОХО
for ($i = 0; $i < count($locations); $i++) { $li = $locations[$i]; doStuff(); doSomeOtherStuff(); dispatch($li); }
На каждом шаге постоянно будет получатся размер массива, а так же опять же создаваться дополнительная переменная.
ХОРОШО
for ($i = 0, $k=count($locations); $i < $k; $i++) { doStuff(); doSomeOtherStuff(); dispatch($locations[$i]); }
Никаких лишних переменных и вычислений в циклах. Как по мне разница между $locations[$i] и $location не играет роли, а вот потери в скорости во втором случае при больших массивах будет существенно.
antonwork
02.09.2017 13:35+3> $locations[$i] и $location не играет роли, а вот потери в скорости во втором случае при больших массивах будет существенно.
ох уж эти микрооптимизации… в плане производительности разницы не будет никакой
win0err
02.09.2017 16:48+4Не стоит забывать, что бы уже не в 2013-м году. Интерпретатор очень сильно поумнел в версии 7.0. И такие микрооптимизации бессмысленны.
Докажу тестом:
Исходный код тестаДоступен на GitHub Gist, вносите правки, если я где-то ошибся.
В тесте измеряем изменение в потребляемой памяти в байтах и время выполнения в микросекундах
<?php echo 'PHP ' . phpversion() . PHP_EOL . PHP_EOL; $test = []; for($i = 0; $i < 100000; $i++) $test[$i] = $i; process_test(function() use ($test) { foreach($test as $t) continue; }); process_test(function() use ($test) { for($a = 0; $a < sizeof($test); $a++) continue; }); process_test(function() use ($test) { for($a = 0, $b = sizeof($test); $a < $b; $a++) continue; }); function process_test($callback) { static $runNumber = 0; $runNumber++; $startedAt = microtime(true); $callback(); echo 'Test #' . $runNumber . ': ' . ( microtime(true) - $startedAt ) . ' seconds' . PHP_EOL; }Travelcamnet
04.09.2017 07:37-3Согласен, что для 7 актуальности не несет. Но, если я не ошибаюсь, доля 5.6 еще достаточно велика, чтобы не забывать о таких вещах)
SerafimArts
06.09.2017 04:15Хочу на всякий случай заметить, что Laravel 5.5 LTS требует 7.0 в минималках, Symfony 4.0 LTS уже 7.1, Doctrine — тоже 7.1, Propel 3 — тоже 7.1, а версия 7.2 на данный момент находится на стадии релиз кандидата. Исходя из этих критериев можно заявлять, что у большинства современных или актуальных проектов на данный момент стоит 7.0 или 7.1.
Доля php 5.6, подозреваю, такая же, как и 5.5, как и 5.4 и прочих, может чуть более. Каждый год образуется некий процент людей, которые живут (написали или поддерживают) в легаси коде и не могут переехать на актуальные версии по этим или иным причинам (ну например у них говнобитрикс или просто лень).
VolCh
02.09.2017 21:19+2Он не будет пересоздавать переменную, он будет перезаписывать её значение, стоимость чего значительно ниже чем стоимость создания, это без учёта возможных оптимизаций в трансляторе
Novikofff
02.09.2017 23:26+2Хорошо:
class UserAuth { private $user; public function __construct($user) { $this->user = user; } protected function verifyCredentials() { // ... } } class UserSettings { private $user; public function __construct($user) { $this->user = $user; $this->auth = new UserAuth($user); } public function changeSettings($settings) { if ($this->auth->verifyCredentials()) { // ... } } }
PSR — Что такое?
DI — И так сойдет)))
Вцелом статья хорошая, но нужно было конечно кому то показать)
VolCh
03.09.2017 10:27Формальное определение: «Если S — это подтип Т, то объекты типа Т могут быть заменены объектами типа S (например, вместо объектов типа Т можно подставить объекты типа S) без изменения каких-либо свойств программы (корректность, задачи и т. д.)». Ещё более пугающее определение.
Можно объяснить проще: если у вас есть родительский и дочерний классы, тогда они могут быть взаимозаменяемы без получения некорректных результатов.Более простое объяснение не соотвествует формальному. Не взаимозаменяемы, а родительский заменяем на дочерний.
Сцепление — очень плохой шаблон разработки, затрудняющий рефакторинг кода.
Сцепление не шаблон, а характеристика, метрика кода.
Kwisatz
03.09.2017 10:53Господа, прошу, не нужно учить весь мир создавать пустые конструкторы.
Это все приводит к тому, что народ делает вещи наподобие:
$lionOptions=['size' => 'big', 'color' => 'black'] $lion=new Lion($lionOptions);
И если в php7 при указании типов это хоть как то переживаемо, то в php5 код превращается в неюзабельное нечто.
И даже если сеттеров не очень много, не очень правильно создавать неполноценных львов, вы не находите?
vlakarados
04.09.2017 10:04+2«Не пишите в глобальные функции» — и сделали синглтон. Сделали ЕЩЕ хуже. Синглтон так же глобален, только без слова глобал.
SerafimArts
04.09.2017 15:52+1Очень часто можно услышать "не используйте ххх" без аргументации "почему не использовать" (да ещё и капслоком накричали). У синглтона две проблемы — это глобальный стейт и скрытые зависимости в местах использования.
Проблема глобалсов же — в неконтролируемости data-flow (плюс всё, перечисленное выше для синглтона). При этом, в случае синглтона мы получем контроль за состоянием, а значит и допустимость stateless, например в тестировании, через какой-нибудь вспомогательный метод
setInstance(?Singleton $instance): void
Остаётся лишь одна проблема, от которой не избавиться — это скрытые зависимости. Но если не переусердствовать с этим, то это не так уж и страшно. В частности — использовать какие-нибудь контроллеры\презентеры в местах применения оного, делегируя его состояние сторонним сервисам. Такие вещи почти всегда тестируются функциональными тестами, а не юнитами, так что фатальных проблем я не наблюдаю.
В любом случае синглтон, максимум для чего может понадобиться — это для DI контейнера. Больше я хз где его можно применять.
Хочется услышать ваше мнение где я ошибаюсь. Почему синглтон хуже глобалсов? Лично я категорически не согласен, это глобалсы на несколько порядков хуже синглтонов.
VolCh
05.09.2017 04:05При этом, в случае синглтона мы получем контроль за состоянием
Можем получить, если постараемся. Но очень часто синглтоны открыты на изменение всем.
SerafimArts
05.09.2017 04:48Можем получить, если постараемся. Но очень часто синглтоны открыты на изменение всем.
Кажется, что для глобального доступа они и предназначены. В любом случае, под "контролем состояния" я подразумевал хотя бы минимальную верификацию данных, а не знание о том что в данный момент содержит синглтон (хотя и это не исключено при должной реализации) и кто его использует (а вот это уже невозможно без магии обратной трассировки, что есть "небольшая дичь").
Но всё равно мой тезис о том, что "глобалсы — это треш и если уж кто-то упоролся до глобального стейта, то лучше уж синглтоны" остаётся в силе =)
ghost404
05.09.2017 15:59Вообще-то вы очень сильно поспешили с переводом.
Там куча косяков. Большую часть я поправил, но например реализация того же LSP не правильная.
А про использованиеSECONDS_IN_A_DAYя вообще молчу.
Если уж сделали перевод, поделитесь с сообществом.
ghost404
05.09.2017 17:18И актуализируйте пожалуйста перевод до текущей версии. Перевод сильно отстал.
Mausglov
05.09.2017 16:23В разъяснении принципа Лисков серьёзная ошибка:
Можно объяснить проще: если у вас есть родительский и дочерний классы, тогда они могут быть взаимозаменяемы без получения некорректных результатов
Они не взаимозаменяемы. Принцип говорит о том, что экземпляр родительского класса можно заменить экземпляром дочернего класса, и всё будет хорошо. В обратную сторону заменять, разумеется, нельзя: смысл дочернего класса в расширении функционала родителя, и если вы возьмёте экземпляр родительского класса и попробуете обратиться к этому расширенному функционалу — вас ждут неприятности.
oxidmod
05.09.2017 16:26Скажите, а почему фигуры не инициализируются через конструктор?
Ведь так намного лучше, не?
abstract class Shape { abstract public function getArea(); } class Rectangle extends Shape { private $width; private $height; public function __construct(float $width, float $height) { $this->width = $width; $this->height = $$height; } public function setWidth(float $width): self { return new self($width, $this->height); } public function setHeight(float $height): self { return new self($this->width, $height); } public function getArea(): float { return $this->width * $this->height; } } class Square extends Shape { private $length; public function __construct(float $length) { $this->length = $length; } public function setLength(float $length): float { return new self($length); } public function getArea() { return $this->length * $this->length; } } function renderShapes(Shape $shapes) { foreach($shapes in $shape) { $area = $shape->getArea(); // render $area }); } $shapes = [new Rectangle(5, 6), new Rectangle(2.5, 10), new Square(3.14)]; renderShapes($shapes);ghost404
05.09.2017 17:10Это сделано для демонстрации проблемы LSP
Есть пример сimmutableклассами.oxidmod
05.09.2017 17:27Да я как-бы не про immutable говорю.
Я о том, что пример ниочемный вцелом.
Что говорит принцип подстановки? что вместо родительского класса может использоваться дочерний и клиентский код не должен от этого зависеть.
В текущей реализации ваш клиентский код зависит от типа. В нем есть проверка на тип, которую чуть выше советовали избегать.
В пулл-реквесте где вместо базового класса используется интерфейс вообще не понятно что. Где использование наследников Shape вперемешку? Там код завязан на конкретные типы.ghost404
05.09.2017 17:41Пример с прямоугольником и квадратом самый популярны при описании принципа LSP.
В PR код завязан на конкретные типы потому, что квадрат и прямоугольник это разные типы. В этом и суть LSP
oxidmod
05.09.2017 17:50Это подтипы Shape, значит код завязанный на Shape должен одинаково работать с Rectangle и Square.
Но у вас нету кода завязанного на Shape. Там не демонстрируется LSP
function areaVerifier(Shape $shape, $expectedArea): bool { return $shape->area() === $expectedArea; }
Вот так будет пример. Клиентский код завязан на абстракцию, и может работать с любым подтипом.ghost404
05.09.2017 18:08Спасибо за замечание. Уже убрал интерфейс
Shape.oxidmod
05.09.2017 18:23рука_лицо.джпег! Если это разные совсем сущности и не являются подтипами одной абстракции то при чем тут LSP совсем???
ghost404
05.09.2017 18:45рука_лицо.джпег! при том что вынесение одной в подтип другой является нарушением LSP, о чем и говорит весь раздел
Если вам не нравится пример, предложите лучше, а не критикуйте.
Это открытый проект.oxidmod
05.09.2017 22:51Внимательно читаем первые 3 абзаца
Перечитываем определением Мартина еще разок или два.
Это тоже самое что написала сама Лисков, но человеческим языком.
Принцип Лисков не о том, что квадрат не нужно наследовать от прямоугольника. Он о программировании на контрактах и о правильном наследовании.
В вашем изначальном примере квадрат унаследованный от прямоугольника замещал логику сеттеров. Это неправильный подход к наследованию. При наследовании реализация должна расширяться, но не замещаться. Если у вас возникла необходимость полностью заменить логику в наследнике, значит этот наслендик никакой на самом деле не наследник.
А теперь сводим все к общему знаменателю. У нас, к примеру, есть задача вывести название фигуры и её площадь. Мы еще даже не знаем какие фигуры мы будем выводить, но мы знаем что нам нужно её название и её площадь. Определим контракт (он же и будет базовым типом по сути):
Например такinterface Shape { public function name(): string; public function area(): float; }ghost404
06.09.2017 00:15Вы не поверите. Я не только Википедию читал, но и ещё с десяток статей по теме.
Но вы похоже не поняли о чем статья и о чем проект.
Вы мне раз за разом пытаетесь объяснить как правильно реализовать LSP.
Задача же проекта показать проблему, то есть пример нарушения LSP, и привести пример решения конкретной проблемы, то есть решение которое не будет нарушать LSP. Как ваше решение, так и моё не нарушает LSP, что и требуется.
Код решат разные задачи потому и правильное решение будет разным.
Ну и авторам больше по нраву другой пример.
oxidmod
06.09.2017 07:24Но должно же быть два примера. Как плохо и как хорошо. К примеру как плохо у меня вопросов нет. Вот унаследовали квадрат от прямоугольника, но клиентский код не может работать с квадратом как с прямоугольником.
Но теперь мы тупо сделали 2 класса независимых и 2 функи для каждого типа и все хорошо? Нет, не хорошо. Фигур много, вам придется под каждую писать свою версию клиентского кода! Вы завязаны на реализацию, а не на абстракцию.
Нарушение LSP не в наследовании, а в том что неправильно выбрана абстракция. Не квадрат подтип прямоугольника. Обе фигуры подтипы абстракции Shape. Тогда можно написать клиентский код завязанный на Shape так, что ему будет не важно какой подтип прилетел.
Как же вы читали то Википедию? Там везде о подтипах и о том, что базовый тип может быть заменен подтипом или подтипы друг другом.
Убрав подтипы вы не сделали хорошо. Вы просто задублировали количество клиентского кода.
oxidmod
06.09.2017 10:14Ну и авторам больше по нраву другой пример.
Даже в этом примере авторы не отказались от наследования. Вы же видите? Они исправили абстракцию, но пингвин по прежнему подтип птицы. Код завязанный на птиц в целом может нарисовать и установить расположение птице и пингвину без любых модификаций.
max_m
решение для «Избегайте проверки типов (часть 1)» будет ещё лучше если интерфейсы использовать:
ghost404
уже используется интерфейс
Traveler