Иначе нет необходимости, и вы можете упростить код для работы без других
Я получил сообщение PHPMD, говорящее мне:
else никогда не требуется, и вы можете упростить код для работы без остального в этой части кода:
if ($settings == null) {
$settings = new self($arrSettings);
} else {
$settings->fill($arrSettings);
}
$settings->save();
return $settings;
Мой вопрос: как я должен избегать else(). Единственный способ, которым я вижу, - это дублирование $setting->save() и вернуться.
Любая идея?
1 ответ
Вероятно, потому что это может быть переписано как
if ($settings === null) {
$settings = new self; // or new self([]);
}
$settings->fill($arrSettings);
$settings->save();
return $settings;
Но, TBH, все это выглядит как одно серьезное нарушение SRP, потому что экземпляры классов не должны быть способны создавать новые экземпляры самих себя. Это просто не имеет никакого смысла... но опять же, я не "ремесленник".
TL,DR
- PHPMD имеет смысл
- Намерения хороши, но PHPMD советует
else"никогда не нужно" неправильно - Вы всегда можете удалить
else- но во многих случаях это приведет к запутыванию кода (что иронично, поскольку вы будете следовать совету "детектора беспорядка"). - Поэтому вы всегда должны спрашивать себя: следует ли мне этого избегать?
else? - Даже сам PHPMD использует
elseтак кажется, что иногда это необходимо
Длинный ответ
else- довольно стандартная языковая конструкция, используемая во многих языковых и программных пакетах (включая сам PHPMD). Мне, говоря, чтоelse никогда не нужно - это то же самое, что сказать, что do...while никогда не нужно, потому что вы можете комбинировать while (true) а также break, что верно, но бессмысленно. Поэтому я бы воспринял этот совет с долей скепсиса и немного подумал, прежде чем менять какое-либо программное обеспечение, основываясь исключительно на рекомендации статического анализатора.
Все сказанное, я думаю, разработчики PHPMD имели в виду, что во многих случаях вы можете и должны удалитьelse из вашего кода, чтобы сделать его более читабельным.
Самые простые случаи:
if ($condition) {
$a = 1;
} else {
$a = 2;
}
Что можно упростить до:
$a = ($condition ? 1 : 2);
Теперь посмотрим на это выражение:
// Calculate using different formulas, depending on $condition.
if ($condition) {
// Calculate using secret formula.
$a = power($r * M_PI, 2) / sqrt(exp($k));
} else {
// Calculate using standard formula.
$a = power(($r / $k) * M_PI, 2) / sqrt(1 / $k);
}
Это можно изменить на:
$a = ($condition ? power($r * M_PI, 2) / sqrt(exp($k)) : power(($r / $k) * M_PI, 2) / sqrt(1 / $k));
Конечно, вторые формы более лаконичны или, я бы сказал, малы. Но с точки зрения ясности кода и удобства обслуживания, я предполагаю, что исходный код сelse гораздо яснее, не говоря уже о том, что объяснять "улучшенные" версии с помощью комментариев кода намного сложнее, не так ли?
ИМХО, это так. И я всегда используюelse в подобных случаях.
Другой простой пример:
// Check animal first.
if ($animal === 'dog') {
if ($breed === 'pinscher') {
$weight = 'light';
} else {
$weight = 'heavy';
}
} else {
// We only deal with dogs.
$weight = "we cannot say anything about $animal";
}
Версия без остального:
$weight = ($animal === 'dog' ? ($breed === 'pinscher' ? 'light' : 'heavy') : "we cannot say anything about $animal");
Обратите внимание, что в этом случае версия без else является прямым нарушением PSR-2, который запрещает вложенные тернарные операторы.
Часто сохраняются простые конструкции, которые в противном случае можно было бы заменить тернарным оператором, просто потому, что вы хотите избежать длинных строк в вашем коде, которые ухудшают читаемость кода:
if (sqrt($weight) > 30 && $animal === 'elephant' && $location == 'zoo') {
$visitors_max = sqrt($guards) / ($ticker_price * M_2_SQRTPI)
} else {
$visitors_max = $visitors_max_last_year * exp($ticket_price) * 1.1;
}
Это становится:
$visitors_max = (sqrt($weight) > 30 && $animal === 'elephant' && $location == 'zoo') ? sqrt($guards) / ($ticker_price * M_2_SQRTPI) : $visitors_max_last_year * exp($ticket_price) * 1.1);
Двигаясь дальше, вот еще один хорошо известный шаблон, который, я полагаю, PHPMD хочет, чтобы вы адресовали:
function myfunc($arg)
{
if ($arg === 'foo') {
$res = 'foo found';
} else {
$len = strlen($arg);
if ($len > 10) {
$res = 'arg is too big';
} else {
$bar = dosomething($res);
$res = "$arg results in $bar";
}
return $res;
}
Эта функция использует совет, который когда-то преподавался в классах программирования, что функции должны иметь единую точку выхода, поскольку это (возможно) облегчает понимание потока программы и поиск ошибок.
IMHO (и PHPMD), мы можем удалить else и улучшить ясность кода / ремонтопригодность в таком коде, ничего не теряя:
function myfunc($arg)
{
if ($arg === 'foo') {
return 'foo found';
}
$len = strlen($arg);
if ($len > 10) {
return 'arg is too big';
}
$bar = dosomething($res);
return "$arg results in $bar";
}
Но это не всегда может быть желательно:
function mycalc($n)
{
if ($n === 0) {
$multiplier = 0.5;
} elseif ($n === 1) {
$multiplier = M_LN2;
} else {
$multiplier = power(sqrt($n * M_PI), 2);
}
return $multiplier * (M_2_PI * power($n * M_PI, 2));
}
"Улучшенная" версия должна выглядеть примерно так:
function mycalc($n)
{
if ($n === 0) {
return 0.5 * (M_2_PI * power($n * M_PI, 2));
}
if ($n === 1) {
return M_LN2 * (M_2_PI * power($n * M_PI, 2));
}
return power(sqrt($n * M_PI), 2) * (M_2_PI * power($n * M_PI, 2));
}
Я не уверен насчет вас, но первая версия гораздо точнее следует моему ходу мыслей о расчетах, поэтому ее гораздо легче понять и поддерживать, чем вторую, даже если в ней используется слово "запрещено" else.
(Кто-то может возразить, что мы могли бы использовать вторую форму плюс вспомогательную переменную для хранения общих вычислений. Достаточно справедливо, но всегда можно возразить, что добавление ненужной переменной делает код менее понятным и дорогостоящим в сопровождении.)
Итак, чтобы ответить на ваш вопрос, как мне избежать другого?, Я еще один спрошу: а зачем вам?
Ответ @tereško имеет некоторый смысл и действительно делает код более лаконичным. Тем не менее, я лично считаю, что ваша первая версия вполне хороша, более ясна по своим намерениям, поэтому намного лучше с точки зрения понятности и ремонтопригодности:
if (I do not have $object)
create a new $object with my settings
else
call the "fill" method of $object with my settings
endif
do stuff with $object
Против:
if (I do not have $object)
create a new $object
endif
call the "fill" method of $object with my settings
do stuff with $object
Также обратите внимание на небольшое изменение логики программирования в версии без elseвыше: вы (и все будущие разработчики) должны предполагать, что вызов метода "fill" объекта $ с моими настройками и создание нового объекта $ с моими настройками всегда приводит к объекту с таким же внутренним состоянием. В исходной версии это предположение не требуется.
Другими словами, отредактированный код будет работать, пока fill() метод и конструктор объекта делают то же самое с внутренним состоянием объекта, которое может быть или не быть истинным - сейчас или когда-либо.
Чтобы проиллюстрировать этот момент, предположим, что объект определен следующим образом:
class MyClass
{
protected $fillCount = 0;
protected $settings;
public function __construct(array $settings)
{
$this->settings = $settings;
}
public function fill(array $settings)
{
$this->fillCount++;
$this->settings = $settings;
}
}
В этом случае ваша исходная версия и версия без else в конечном итоге появятся объекты с разными внутренними состояниями, и найти ошибку будет немного сложнее, потому что она будет скрыта за предположениями и неявными конструкциями.
Теперь давайте посмотрим на один собственный PHPMD else:
// File: src/bin/phpmd
if (file_exists(__DIR__ . '/../../../../autoload.php')) {
// phpmd is part of a composer installation
require_once __DIR__ . '/../../../../autoload.php';
} else {
require_once __DIR__ . '/../../vendor/autoload.php';
// PEAR installation workaround
if (strpos('@package_version@', '@package_version') === 0) {
set_include_path(
dirname(__FILE__) . '/../main/php' .
PATH_SEPARATOR .
dirname(__FILE__) . '/../../vendor/pdepend/pdepend/src/main/php' .
PATH_SEPARATOR .
'.'
);
}
}
// (...100+ lines of code follows...)
Вопрос в том, стоит ли этого избегать?else?
На момент написания, чтобы сделать phpmd счастливым, просто используйте elseif вместо тогоelse (имеет смысл с точки зрения удобочитаемости, когда тело первого if длинный)
$condition = ... ;
if ($condition) {
...
} elseif (!$condition) {
...
}