Итак, очередная статья из цикла "Хороший, качественный код". На этот раз мне бы хотелось поговорить об оформлении условного оператора if. Этот оператор присутствует почти в каждой программе, и им можно пользоваться по-разному. Как и раньше, не буду претендовать на то, что мои слова - истина в последней инстанции, и предоставлю несколько возможностей выбора.
Начнем с форматирования исходников:
На какой строчке ставить фигурную скобку?
Вообще говоря, существует две старые конвенции (см. иллюстрацию ниже):
| if (a == b) { // do something } | if (a == b) { // do something } |
Первый вариант в последнее время часто уходит в небытие: продукты Microsoft предпочитают вторую конвенцию, Adobe Flex Builder предпочитает вторую конвенцию... Следовательно, многие программисты на платформе Windows предпочитают вторую конвенцию (фигурная скобка на следующей строчке). Лично я - за первый вариант (даже если он противоречит официальным guidelines разработчика), потому что:
- широкоформатные мониторы это модно. Чем меньше строчек в высоту, тем лучше
- распечатка кода на принтере устарела, но не отмерла. Иногда я смотрю на исходники с листа бумаги, и такой вариант куда привлекательнее
- code folding (сворачивание кода) это тоже модно. Первый вариант сворачивается в одну строчку (даже если после условия есть комментарий), второй вариант сворачивается в две строки (а комментарий пропадет).
Какой из вариантов написания выбрать - решать, конечно, вам. В некоторых организациях существуют общепринятые стандарты - если таковые есть, - несомненно, нужно следовать им. В случае, если вами используется какая-нибудь хитрая среда разработки (вроде IntelliJ IDEA), редактор может сам переформатировать исходник за вас.
Нужно ли обрамлять в фигурные скобки одиночную команду?
| if ( o == null ) throw new Exception(); | if ( o == null ) { throw new Exception(); } | if ( o == null ) throw new Exception(); |
В данном примере, мне симпатичнее всего вариант №1. Во-первых, своим форматированием он подчеркивает, что создание Exception'а - это логический подблок. Во-вторых, он написан кратко. Второй вариант - слишком длинный. Третий вариант - просто недостаточно четкий.
Но варианта 1 придерживаться следует далеко не всегда. Например, если условие действительно сложное:
| if ( a == b ) { throw new Exception(); } else { c = a + b; d = a - b; } | if ( a == b ) throw new Exception(); else { c = a + b; d = a - b; } |
В данном случае, код из варианта 1 смотрится более целостно. Когда хотя бы одна из веток оператора if содержит в себе сложную логику, я предпочитаю обрамлять все блоки в фигурные скобки.
Мешанина бизнес-логики с отображением
Некоторые языки, такие как PHP, вообще очень способствуют написанию непонятного кода. Например, в одном из проектов я видел примерно такой код. Далее до конца поста, примеры написаны на псевдокоде, как две капли воды похожем на PHP:
[geshi lang=PHP]if (isset($_REQUEST['money'])) {
if ( $money Недостаточно денег на счету";
} else {
// выполняем нашу работу
print "Покупка выполнена";
}
} else {
print "Не указана сумма";
}
[/geshi]
Или, в нашей собственной админке, в старом PHP-коде до сих пор содержится вот что:
[geshi lang=PHP]$succmsg = '';
$errmsg = '';
if ($admin && (isset($_REQUEST["user"]) && $user=$_REQUEST["user"])) {
if (isset($_REQUEST['newpassword']) && $newpass = $_REQUEST['newpassword']) {
if (!preg_match('^[_a-zA-Z0-9]+$', $newpassword)) {
$errmsg = "Bad password";
$newpass = '';
}
}
elseif ($newemail != '') {
if (substr($res, 0, 2) == "OK") {
$succmsg = "E-mail changed successfully";
} else {
$errmsg = "Cannot change email";
}
}
elseif ($newpass != '') {
if (substr($res, 0, 2) == "OK") {
$succmsg = $res;
} else {
$errmsg = "Cannot change password";
}
}
}
// do something
if ($succmsg != '') {
print "".$succmsg."\n";
}
elseif ($errmsg != '') {
print "".$errmsg."\n";
}
[/geshi]
Код, который выполняет соответствующие функции, опущен - он нам неинтересен. Вообще это чудо из нашей админки умеет менять пользователю почтовый ящик либо пароль. iMHO, выглядит уже лучше, чем предыдущий вариант, но все еще неважнецки.
Перепишем этот код таким образом, чтобы не использовалось сложных ветвлений. Итак:
[geshi lang=PHP]$succmsg = '';
if ($admin && (isset($_REQUEST["user"]) && $user=$_REQUEST["user"])) {
if (isset($_REQUEST['newpassword']) && $newpass = $_REQUEST['newpassword']) {
if (!preg_match('^[_a-zA-Z0-9]+$', $newpassword))
throw new Exception("Bad password");
}
elseif ($newemail != '') {
if (substr($res, 0, 2) != "OK")
throw new Exception("Cannot change email");
$succmsg = "E-mail changed successfully";
}
elseif ($newpass != '') {
if (substr($res, 0, 2) != "OK")
throw new Exception("Cannot change password");
$succmsg = $res;
}
}
try {
// Вызываем вышеописанный метод
print "".$succmsg."\n";
}
catch (Exception $e) {
print "".$e->getMessage()."\n";
}[/geshi]
Мы получили:
- в отличие от примера 1, нет нужды каждый раз оформлять сообщение об ошибке
- в отличие от примера 2, мы получили более компактный код
Более того, за счет использования Exception, мы можем идентифицировать место, где произошла ошибка. Для этого, разработчику достаточно нужно выдать не print $e->getMessage(), а просто print $e. Такой код проще интернационализировать (да, мир, к сожалению, не ограничивается только русским языком). Такие сообщения об ошибках можно продублировать в базу данных (не дублируя сам код).
Можно ли уменьшить количество вложенных условных операторов?
Временами, когда код требует наличия большого количества проверок, очень хочется использовать большое количество вложенных if:
[geshi lang=PHP]
if (isset($_REQUEST['a']) && $a = $_REQUEST['a'])) {
if (isset($_REQUEST['b']) && $b = $_REQUEST['a'])) {
return $a + $b;
}
}
return false;
[/geshi]
Но что будет, если условий станет больше?
[geshi lang=PHP]
if (isset($_REQUEST['a']) && $a = $_REQUEST['a'])) {
if (isset($_REQUEST['b']) && $b = $_REQUEST['a'])) {
if (isset($_REQUEST['c']) && $c = $_REQUEST['c'])) {
if (isset($_REQUEST['d']) && $d = $_REQUEST['d'])) {
return $a + $b + $c + $d;
}
}
}
}
return false;
[/geshi]
Возможно, в таком случае лучше сделать более длинно, но и более понятно:
[geshi lang=PHP]if (!isset($_REQUEST['a']) return false; if (!isset($_REQUEST['b']) return false; if (!isset($_REQUEST['c']) return false; if (!isset($_REQUEST['d']) return false; $a = $_REQUEST['a']; $b = $_REQUEST['b']; $c = $_REQUEST['c']; $d = $_REQUEST['d']; return $a + $b + $c + $d; [/geshi]
Или, еще лучше:
[geshi lang=PHP]
function getParam($param) {
if (!isset($_REQUEST[$param]))
throw new Exception("Parameter $param is missing");
return $_REQUEST[$param];
}
try {
$a = getParam('a');
$b = getParam('b');
$c = getParam('c');
$d = getParam('d');
return $a + $b + $c + $d;
}
catch (Exception $e) {
return false;
}[/geshi]
Резюме
- Оформление условных операторов if во всем проекте должно быть одинаковым. Я предпочитаю однострочные операторы не брать в фигурные скобки. Если хотя бы одна из веток if - многострочная, то в фигурные скобки берутся все ветки. Это создает преимущества при просмотре исходников на широкоформатных мониторах и в распечатанном виде, а также при использовании Code Folding.
- Бизнес-логику следует стараться отделить от презентационной части. В частности, может помочь использование Exception'ов. Использование Exception позволяет выдавать больше информации при просмотре ошибки разработчиком, упростить перевод сообщений об ошибках на другие языки, упростить занесение ошибок в базу данных. Наконец, размер программы сокращается, а ее текст становится более понятным.
- Лучше по возможности сокращать многократно вложенные операторы if. Для этого, опять-таки, могут использоваться Exceptionы.