[Из песочницы] Эстетическая красота: Switch vs If
Вводная
Как разработчики, мы каждый день сталкиваемся с кодом и чем больше того, который приходится нам по душе, мы видим, пишем, тем большим энтузиазмом проникаемся, тем более продуктивными и эффективными становимся. Да что там говорить, мы просто гордимся нашим кодом. Но одна дилемма не дает мне покоя: когда 2 разработчика смотрят на один и тот же код они могут испытывать совершенно противоположные чувства. И что делать, если эти чувства, эмоции, навеянные его эстетической красотой, не совпадает с эмоциями большинства окружающих Вас профессионалов? В общем, история о том, почему может не нравиться языковая конструкция switch на столько, что предпочитаешь if. Кому интересна эта холиварная позиция добро пожаловать под кат.
Немного нюансов
Речь ниже пойдет о сравнении оператора switch и его конкретной реализаций на языке Java и операторами if в виде оппонентов этой конструкции. О том, что у оператора switch имеется туз в рукаве — производительность (за редким исключением), пожалуй, знает каждый и этот момент рассматриваться не будет — так как крыть его нечем. Но все же, что не так?
1. Многословность и громоздкость
Среди причин, отталкивающих меня от использования оператора switch — это многословность и громоздкость самой конструкции, только взгляните — switch, case, break и default, причем опускаю за скобки, что между ними, наверняка, еще встретится return и throw. Не подумайте, что призываю не использовать служебные слова языка java или избегать их большого количества в коде, нет, лишь считаю, что простые вещи не должны быть многословными. Вот небольшой пример того, о чем говорю:
public List getTypes1(Channel channel) {
switch (channel) {
case INTERNET:
return Arrays.asList("SMS", "MAC");
case HOME_PHONE:
case DEVICE_PHONE:
return Arrays.asList("CHECK_LIST");
default:
throw new IllegalArgumentException("Неподдерживаемый канал связи " + channel);
}
}
вариант с if
public List getTypes2(Channel channel) {
if (INTERNET == channel) {
return Arrays.asList("SMS", "MAC");
}
if (HOME_PHONE == channel || DEVICE_PHONE == channel) {
return Arrays.asList("CHECK_LIST");
}
throw new IllegalArgumentException("Неподдерживаемый канал связи " + channel);
}
Итого для меня: switch/case/default VS if.
Если спросить, кому какой нравится больше код, то большинство отдаст предпочтение 1ому варианту, в то время как по мне, он — многословный. Речь о рефакторинге кода здесь не идет, все мы знаем, что можно было бы использовать константу типа EnumMap или IdentityHashMap для поиска списка по ключу channel или вообще убрать нужные данные в сам Channel, хотя это решение дискуссионное. Но вернемся.
2. Отступы
Возможно в академических примерах использование оператора swicth — это единственная часть кода, но тот код, с которым приходится сталкиваться, поддерживать — это более сложный, обросший проверками, хаками, где-то просто предметной сложностью код. И лично меня, каждый отступ в таком месте напрягает. Обратимся к 'живому' примеру (убрал лишнее, но суть осталась).
public static List getReference1(Request request, DataSource ds) {
switch (request.getType()) {
case "enum_ref":
return EnumRefLoader.getReference(ds);
case "client_ref":
case "any_client_ref":
switch (request.getName()) {
case "client_types":
return ClientRefLoader.getClientTypes(ds);
case "mailboxes":
return MailboxesRefLoader.getMailboxes(ds);
default:
return ClientRefLoader.getClientReference(request.getName(), ds);
}
case "simple_ref":
return ReferenceLoader.getReference(request, ds);
}
throw new IllegalStateException("Неподдерживаемый тип справочника: " + request.getType());
}
вариант с if
public static List getReference2(Request request, DataSource ds) {
if ("enum_ref".equals(request.getType())) {
return EnumRefLoader.getReference(ds);
}
if ("simple_ref".equals(request.getType())) {
return ReferenceLoader.getReference(request, ds);
}
boolean selectByName = "client_ref".equals(request.getType()) || "any_client_ref".equals(request.getType());
if (!selectByName) {
throw new IllegalStateException("Неподдерживаемый тип справочника: " + request.getType());
}
if ("client_types".equals(request.getName())) {
return ClientRefLoader.getClientTypes(ds);
}
if ("mailboxes".equals(request.getName())) {
return MailboxesRefLoader.getMailboxes(ds);
}
return ClientRefLoader.getClientReference(request.getName(), ds);
}
Итого для меня: 5 отступов VS 2.
Но опять, кому какой нравится вариант? Большинство отдаст предпочтение getReference1.
Отдельно стоит отметить, что количество отступов еще зависит от выбранного стиля форматирования кода.
3. Проверка на null
Если используется оператор switch со строками или енумами параметр выбора необходимо проверять на null. Вернемся к getTypes примерам.
// switch: проверка на null нужна
public List getTypes1(Channel channel) {
// Проверка на null
if (channel == null) {
throw new IllegalArgumentException("Канал связи должен быть задан");
}
switch (channel) {
case INTERNET:
return Arrays.asList("SMS", "MAC");
case HOME_PHONE:
case DEVICE_PHONE:
return Arrays.asList("CHECK_LIST");
default:
throw new IllegalArgumentException("Неподдерживаемый канал связи " + channel);
}
}
// if: можно обойтись без проверки на null
public List getTypes2(Channel channel) {
if (INTERNET == channel) {
return Arrays.asList("SMS", "MAC");
}
if (HOME_PHONE == channel || DEVICE_PHONE == channel) {
return Arrays.asList("CHECK_LIST");
}
throw new IllegalArgumentException("Неподдерживаемый канал связи " + channel);
}
Итого для меня: лишний код.
Даже если Вы абсолютно уверенны сейчас в том, что null 'не придет', это абсолютно не значит, что так будет всегда. Я проанализировал корпоративный багтрэк и нашел этому утверждению подтверждение. Справедливости ради стоит отметить, что структура кода выраженная через if не лишена этой проблемы, зачастую константы для сравнения используются справа, а не слева, например, name.equals («John»), вместо «John».equals (name). Но в рамках данной статьи, по этому пункту, хотел сказать, что при прочих равных, подход со switch раздувается проверкой на null, при if же проверка не нужна. Добавлю еще, что статические анализаторы кода легко справляются с возможными null-багами.
4. Разношерстность
Очень часто, при длительном сопровождении кода, кодовая база раздувается и можно легко встретить код, похожий на следующий:
public static void doSomeWork(Channel channel, String cond) {
Logger log = getLogger();
//...
switch (channel) {
//...
case INTERNET:
// Со временем появляется такая проверка
if ("fix-price".equals(cond)) {
// ...
log.info("Your tariff");
return;
}
// Еще под выбор?
// ...
break;
//...
}
//...
}
Итого для меня: разный стиль.
Раньше был 'чистенький' switch, а теперь switch + if. Происходит, как я называю, смешение стилей, часть кода 'выбора' выражена через switch, часть через if. Разумеется никто не запрещает использовать if и switch вместе, если это не касается операции выбора/под выбора, как в приведенном примере.
5. Чей break?
При использовании оператора switch, в case блоках может появиться цикл или на оборот, в цикле — switch, со своими прерываниями процесса обработки. Вопрос, чей break, господа?
public static List whoseBreak(List states) {
List results = new ArrayList<>();
for (String state : states) {
Result result = process(state);
switch (result.getCode()) {
case "OK":
if (result.hasId()) {
results.add(result.getId());
// Надуманный случай, но break чей-то?
break;
}
if (result.getInnerMessage() != null) {
results.add(result.getInnerMessage());
// Вот это поворот
continue;
}
// ...
break;
case "NOTHING":
results.add("SKIP");
break;
case "ERROR":
results.add(result.getErroMessage());
break;
default:
throw new IllegalArgumentException("Неверный код: " + result.getCode());
}
}
return results;
}
Итого для меня: читаемость кода понижается.
Если честно, встречались и куда более запутанные примеры кода.
6. Неуместность
В java 7 появилась возможность использовать оператор switch со строками. Когда наша компания перешла на java 7 — это был настоящий Switch-Бум. Может по этому, а может и по другой причине, но во многих проектах встречаются похожие заготовки:
public String resolveType(String type) {
switch (type) {
case "Java 7 же?":
return "Ага";
default:
throw new IllegalArgumentException("Люблю switch, жаль, что нет такого варианта с case " + type);
}
}
Итого для меня: появляются неуместные конструкции.
7. Гламурный switch под аккомпанемент хардкода
Немного юмора не помешает.
public class Hit {
public static enum Variant {
ZERO, ONE, TWO, THREE
}
public static void switchBanter(Variant variant) {
int shift = 0;
ZERO: ONE: while (variant != null) {
shift++;
switch (variant) {
default: {
THREE: {
System.out.println("default");
break THREE;
}
break;
}
case ONE: {
TWO: {
THREE: for (int index = shift; index <= 4; index++) {
System.out.println("one");
switch (index) {
case 1: continue ONE;
case 2: break TWO;
case 3: continue THREE;
case 4: break ZERO;
}
}
}
continue ONE;
}
case TWO: {
TWO: {
System.out.println("two");
if (variant == THREE) {
continue;
}
break TWO;
}
break ZERO;
}
}
variant = null;
}
}
public static void main(String[] args) {
switchBanter(ONE);
}
}
Без комментариев.
Заключение
Я не призываю Вас отказываться от оператора switch, местами он действительно хорош собой и лапша из if/if-else/else та еще каша. Но черезмерное его использование где ни попадя, может вызывать недовольство у других разработчиков. И я один из них.
Отдельно хотелось отметить, что c точки зрения понимания кода, у меня нет никаких проблем с switch/case — смысл написанного ясен, но вот с точки зрения восприятия эстетической красоты — есть.
И напоследок. Используйте то, что Вам приходится по душе, опуская за скобки навязанные мнения, главное, чтобы Ваш код был простым, рабочим, красивым и надежным. Всего хорошего.
Комментарии (29)
Sirikid
29 января 2017 в 00:44 (комментарий был изменён)
+7↑
↓
А как вам такой финт?
fun getTypes1(channel: Channel): List= when (channel) { INTERNET -> listOf("SMS", "MAC") HOME_PHONE, DEVICE_PHONE -> listOf("CHECK_LIST") else -> throw IllegalArgumentException("Неподдерживаемый канал связи " + channel) } 29 января 2017 в 12:34
+1↑
↓
Финт прекрасен: краток и лаконичен.
Косвенно Вы поднимаете более глубокую тему сравнения языков и их возможностей.-
Sirikid
29 января 2017 в 16:37
+1↑
↓
Скорее тему бездумного заимствования конструкций при проектировании языков, когда проектировалась Java аналоги
whenуже были.
fogone
29 января 2017 в 19:14
+1↑
↓
Ещё интерполяция для формирования сообщения нужна
AlexZaharow
29 января 2017 в 00:49
+5↑
↓
Подброшу дровишек. Есть такая штука, как «java break label»:public class Test { public static void main(String[] argv) { lab1: for (int i=0; i<10; i++) { for (int j=0; j<10; j++) { if (j>=5 && i>5) break lab1; System.out.println(i + " / " + j); } } } }
Не часто пользуюсь, но в принципе местами бывает удобна. На if-ах это сделать уже значительно сложнее без дополнительных переменных.
Тоже самое есть и в JavaScript: https://developer.mozilla.org/ru/docs/Web/JavaScript/Reference/Statements/labelvar i, j; loop1: for (i = 0; i < 3; i++) { //Первый цикл, обозначенный меткой "loop1" loop2: for (j = 0; j < 3; j++) { //Второй цикл, обозначенный меткой "loop2" if (i == 1 && j == 1) { continue loop1; } console.log("i = " + i + ", j = " + j); } }
TheShock
29 января 2017 в 06:28
+6↑
↓
Обычно в таких случаях лучше подумать о рефакторинге, а не о лейбле.
charypopper
29 января 2017 в 12:26
+6↑
↓
Хорошая попытка goto, но нет)
29 января 2017 в 12:42
+1↑
↓
На if-ах это сделать уже значительно сложнее без дополнительных переменных.
Согласен, но тоже верно и про switch-case?
29 января 2017 в 02:59
+7↑
↓
ИМХО проблемы надуманные. Большой блок switch всегда можно разбить на отдельные функции, а в маленьком всё вышеописанное не проявляется. Громоздкость, вложенность, отступы, разношерстность, путаница с break — всё это решается декомпозицией, особенно в switch, который by design разбит на независимые участки кода. Он прямо так и напрашивается на декомпозицию.
Отдельно можно сказать про проверку на null. Она нужна в любом случае, т.к. передача null в метод это, как правило, ошибка, и об этом надо сообщить. Что бы ни было внутри метода, switch, if, или просто «Hello World», а проверка входных данных должна быть.29 января 2017 в 08:25
+2↑
↓
Бывает так, что в таком случае надо делать функции с 10-ю входными и 5-ю выходными переменными, причём в каждом case разными, и с javadoc длиннее кода раз в 5. И это только ещё сильнее всё запутывает.29 января 2017 в 12:32
+5↑
↓
Если ты не пишешь на языке низкого уровня, то появление функции «с 10-ю входными и 5-ю выходными переменными» говорит только об ужасном дизайне. Тут уже не о выборе меджу switch и if говорить надо.
29 января 2017 в 13:17
+2↑
↓
ИМХО проблемы надуманные
Я всячески старался избегать слова проблема, больше старался подчеркивать личная неприязнь, но справдливости ради скажу, что проблемы if VS switch находятся при review кода, как в одну так и в другую стороны.Отдельно можно сказать про проверку на null. Она нужна в любом случае, т.к. передача null в метод …
Бесспорно! Но жизнь такова, что многие проверки не делают и в результате получаем, в одном случае крайне неинформативную ошибку NPE, а в другом просто недостаточно информативную (если из примера «Неподдерживаемый канал связи null»)
29 января 2017 в 03:49
+6↑
↓
В случаях когда switch больше семантически соответствует алгоритму правильнее использовать такую конструкцию.Во-первых, тогда человеку разбирая алгоритм не нужно искать второй и последующий if чтобы понять, что это не просто проверка, а выбор из альтернатив.
Во-вторых, компилятор или интерпретатор смогут лучше оптимизировать свою работу если код точнее следует алгоритму. Используя серию if вы подменяете алгоритм одной из возможных его реализаций и заменяете одну конструкцию целой серией других, то есть усложняете алгоритм.Однако ничто не заставит писать код «как правильно», когда хочется «как хочется».
maaGames
29 января 2017 в 06:40
+9↑
↓
У if-else есть один большой недостаток: ошибка копипасты. В switch невозможно по ошибке написать два одинаковых case, в if-else это случается постоянно.
А в целом, всему своё место.29 января 2017 в 13:21
+1↑
↓
Голосую +1. Лично на это нарывался.
Borz
29 января 2017 в 14:37
+1↑
↓
в нормальных IDE подобные ошибки подсвечиваются
-
maaGames
29 января 2017 в 15:11
+1↑
↓
Вы про if? Возможно, зависит от языка, но на С/С++ такие ошибки только PVS и прочие статические анализаторы находят, сама IDE бессильна. 
PsyHaSTe
29 января 2017 в 18:41
+1↑
↓
А как будете ловить ошибку, что вместо одной переменной сравнили с другой?public ListgetTypes2(Channel channel) { Channel modifiedChannel = DoSomethingWith(channel); if (INTERNET == modifiedChannel) { return Arrays.asList("SMS", "MAC"); } if (HOME_PHONE == modifiedChannel || DEVICE_PHONE == channel) { // <- сравниваем не ту ссылку return Arrays.asList("CHECK_LIST"); } throw new IllegalArgumentException("Неподдерживаемый канал связи " + channel); }
Ни один статический анализатор тут ошибку не найдет.-
Borz
29 января 2017 в 18:47
+1↑
↓
зато тесты найдут. либо через юнит либо через мутацию
-
PsyHaSTe
29 января 2017 в 19:32
+1↑
↓
Ну то есть ответ «никак, покрою тестами».-
Borz
29 января 2017 в 19:43
0↑
↓
да.
P.S. вы подменили смысл моего первого комментария — одно дело, когда в последующем if есть условие ранее проверенное другим if (например следствие copy-paste) и совсем другое неявная логическая ошибка в условиях (может вам именно такая проверка и нужна была)
-
Borz
29 января 2017 в 10:01
+1↑
↓
в силу предпочтений линейного кода против древовидного внутри одного метода, для себя давно решил, что switch только в тех случаях, когда внутри нет return — в остальных случаях строго if.
Да и чаще всего, логика несколько сложнее чем просто проверка 1-in-1 константы, и тут снова if в выигрыше 29 января 2017 в 10:50
+1↑
↓
Я обычно избегаю обоих конструкций, и инкапсулирую логику в отдельный метод в enum.
И тогда выходит очень аккуратно.ConnectionType connectionType = ConnectionType.LIST;
return connectionType.getRows ();У каждого элемента enum своя реализация метода. И проверка на null не нужна, и код читабельнее.
29 января 2017 в 15:17
+1↑
↓
А ещё гудбай статически просчитанный диспатч. Или надеяться на JIT.
29 января 2017 в 15:26
+1↑
↓
Если в выражениях switch логика сложнее константы, ваш аргумент теряет смысл.
29 января 2017 в 15:30
+1↑
↓
А Java умеет в метках диспатч-веток сложные неконстантные выражения?
29 января 2017 в 16:49 (комментарий был изменён)
+1↑
↓
Для case требуются только константы. Но я не об этом.
Вы говорите о том, что в случае использования switch-case есть вероятность того, что компилятор сможет просчитать, куда передать управление дальше.
Если я правильно вас понял, и не ошибаюсь, это возможно в случае, если в ответах case будут константы:case 4: retrurn "some";
В случае более сложной логики вродеcase 4: if (someFlag) service.run(something)
говорить о статическом диспатче не приходится.Но если честно, я давно не встречал switch-case, который возвращает константу.
Подскажите примером, для чего это может требоваться?
29 января 2017 в 12:37
+1↑
↓
На сколько я помню, в первобытных сях оператор «свитч» был предназначен для ассемблера, в котором есть схожий оператор, обладающий лучшим быстродействием чем цепочка сравнений (условий)29 января 2017 в 15:16
+1↑
↓
Скорее всего побуду Кэпом. Это называется Dispatch Table и напрямую в Асм вроде как не выражается. А выражается в джамп с вычисляемым адресом, перед которым стоит проверка что джамп не улетит в космос.
