Иногда условия могут становиться довольно сложными, поэтому для удобства чтения я обычно делю их и даю каждому компоненту осмысленное имя. Однако это отрицательно сказывается на оценке короткого замыкания, что может создать проблему. Я придумал подход обертки, но, на мой взгляд, он слишком многословен.
Может кто-нибудь придумать подходящее решение для этого?
См. Код ниже для примеров того, что я имею в виду:
public class BooleanEvaluator {
// problem: complex boolean expression, hard to read
public static void main1(String[] args) {
if (args != null && args.length == 2 && !args[0].equals(args[1])) {
System.out.println("Args are ok");
}
}
// solution: simplified by splitting up and using meaningful names
// problem: no short circuit evaluation
public static void main2(String[] args) {
boolean argsNotNull = args != null;
boolean argsLengthOk = args.length == 2;
boolean argsAreNotEqual = !args[0].equals(args[1]);
if (argsNotNull && argsLengthOk && argsAreNotEqual) {
System.out.println("Args are ok");
}
}
// solution: wrappers to delay the evaluation
// problem: verbose
public static void main3(final String[] args) {
abstract class BooleanExpression {
abstract boolean eval();
}
BooleanExpression argsNotNull = new BooleanExpression() {
boolean eval() {
return args != null;
}
};
BooleanExpression argsLengthIsOk = new BooleanExpression() {
boolean eval() {
return args.length == 2;
}
};
BooleanExpression argsAreNotEqual = new BooleanExpression() {
boolean eval() {
return !args[0].equals(args[1]);
}
};
if (argsNotNull.eval() && argsLengthIsOk.eval() && argsAreNotEqual.eval()) {
System.out.println("Args are ok");
}
}
}
Ответ на ответы:
Спасибо за все ваши идеи! На данный момент были представлены следующие альтернативы:
- Разбивать строки и добавлять комментарии
- Оставить как есть
- Методы извлечения
- Ранний возврат
- Вложенный / Разделить, если это
Разбивать строки и добавлять комментарии:
Простое добавление разрывов строк в условие отменяется средством форматирования кода в Eclipse (ctrl + shift + f). Встроенные комментарии помогают в этом, но оставляют мало места в каждой строке и могут привести к некрасивому переносу. Однако в простых случаях этого может быть достаточно.
Оставить как есть:
Условие примера, которое я привел, является довольно упрощенным, поэтому вам, возможно, не потребуется решать проблемы с читабельностью в этом случае. Я думал о ситуациях, когда условие намного сложнее, например:
private boolean targetFound(String target, List<String> items,
int position, int min, int max) {
return ((position >= min && position < max && ((position % 2 == 0 && items
.get(position).equals(target)) || (position % 2 == 1)
&& position > min && items.get(position - 1).equals(target)))
|| (position < min && items.get(0).equals(target)) || (position >= max && items
.get(items.size() - 1).equals(target)));
}
Я бы не рекомендовал оставлять это как есть.
Методы извлечения:
Я рассмотрел методы извлечения, как было предложено в нескольких ответах. Недостатком этого является то, что эти методы обычно имеют очень низкую степень детализации и сами по себе могут быть не очень значимыми, поэтому они могут загромождать ваш класс, например:
private static boolean lengthOK(String[] args) {
return args.length == 2;
}
Это на самом деле не заслуживает того, чтобы быть отдельным методом на уровне класса. Также вы должны передать все соответствующие аргументы каждому методу. Если вы создаете отдельный класс исключительно для оценки очень сложного условия, тогда это может быть хорошим решением IMO.
То, чего я пытался достичь с помощью подхода BooleanExpression, - это то, что логика остается локальной. Обратите внимание, что даже объявление BooleanExpression является локальным (я не думаю, что когда-либо сталкивался с вариантом использования для объявления локального класса!).
Раннее возвращение:
Решение о досрочном возвращении кажется адекватным, хотя я не одобряю эту идиому. Альтернативное обозначение:
public static boolean areArgsOk(String[] args) {
check_args: {
if (args == null) {
break check_args;
}
if (args.length != 2) {
break check_args;
}
if (args[0].equals(args[1])) {
break check_args;
}
return true;
}
return false;
}
Я понимаю, что большинство людей ненавидят ярлыки и разрывы, и этот стиль может быть слишком необычным, чтобы считаться читабельным.
Вложено / разделено, если:
Позволяет вводить значимые имена в сочетании с оптимизированной оценкой. Недостатком является сложное дерево условных операторов, которое может следовать
Showdown
Таким образом, чтобы увидеть, какой подход я полностью одобряю, я применил несколько предложенных решений к сложному примеру targetFound, представленному выше. Вот мои результаты:
вложенные / разбитые if со значимыми именами
очень многословные, значимые имена не очень помогают читабельности здесь
private boolean targetFound1(String target, List<String> items,
int position, int min, int max) {
boolean result;
boolean inWindow = position >= min && position < max;
if (inWindow) {
boolean foundInEvenPosition = position % 2 == 0
&& items.get(position).equals(target);
if (foundInEvenPosition) {
result = true;
} else {
boolean foundInOddPosition = (position % 2 == 1)
&& position > min
&& items.get(position - 1).equals(target);
result = foundInOddPosition;
}
} else {
boolean beforeWindow = position < min;
if (beforeWindow) {
boolean matchesFirstItem = items.get(0).equals(target);
result = matchesFirstItem;
} else {
boolean afterWindow = position >= max;
if (afterWindow) {
boolean matchesLastItem = items.get(items.size() - 1)
.equals(target);
result = matchesLastItem;
} else {
result = false;
}
}
}
return result;
}
вложено / разделено, если с комментариями
менее многословно, но все еще трудно читать и легко создавать ошибки
private boolean targetFound2(String target, List<String> items,
int position, int min, int max) {
boolean result;
if ((position >= min && position < max)) { // in window
if ((position % 2 == 0 && items.get(position).equals(target))) {
// even position
result = true;
} else { // odd position
result = ((position % 2 == 1) && position > min && items.get(
position - 1).equals(target));
}
} else if ((position < min)) { // before window
result = items.get(0).equals(target);
} else if ((position >= max)) { // after window
result = items.get(items.size() - 1).equals(target);
} else {
result = false;
}
return result;
}
досрочный возврат
еще более компактно, но условное дерево остается таким же сложным
private boolean targetFound3(String target, List<String> items,
int position, int min, int max) {
if ((position >= min && position < max)) { // in window
if ((position % 2 == 0 && items.get(position).equals(target))) {
return true; // even position
} else {
return (position % 2 == 1) && position > min && items.get(
position - 1).equals(target); // odd position
}
} else if ((position < min)) { // before window
return items.get(0).equals(target);
} else if ((position >= max)) { // after window
return items.get(items.size() - 1).equals(target);
} else {
return false;
}
}
извлеченные методы
приводит к бессмысленным методам в вашем классе
передача параметров раздражает
private boolean targetFound4(String target, List<String> items,
int position, int min, int max) {
return (foundInWindow(target, items, position, min, max)
|| foundBefore(target, items, position, min) || foundAfter(
target, items, position, max));
}
private boolean foundAfter(String target, List<String> items, int position,
int max) {
return (position >= max && items.get(items.size() - 1).equals(target));
}
private boolean foundBefore(String target, List<String> items,
int position, int min) {
return (position < min && items.get(0).equals(target));
}
private boolean foundInWindow(String target, List<String> items,
int position, int min, int max) {
return (position >= min && position < max && ((position % 2 == 0 && items
.get(position).equals(target)) || (position % 2 == 1)
&& position > min && items.get(position - 1).equals(target)));
}
Оболочки BooleanExpression повторно
обратите внимание, что параметры метода должны быть объявлены как окончательные
для этого сложного случая многословие оправдано ИМО
Может быть, закрытия сделают это проще, если они когда-нибудь согласятся с этим (-
private boolean targetFound5(final String target, final List<String> items,
final int position, final int min, final int max) {
abstract class BooleanExpression {
abstract boolean eval();
}
BooleanExpression foundInWindow = new BooleanExpression() {
boolean eval() {
return position >= min && position < max
&& (foundAtEvenPosition() || foundAtOddPosition());
}
private boolean foundAtEvenPosition() {
return position % 2 == 0 && items.get(position).equals(target);
}
private boolean foundAtOddPosition() {
return position % 2 == 1 && position > min
&& items.get(position - 1).equals(target);
}
};
BooleanExpression foundBefore = new BooleanExpression() {
boolean eval() {
return position < min && items.get(0).equals(target);
}
};
BooleanExpression foundAfter = new BooleanExpression() {
boolean eval() {
return position >= max
&& items.get(items.size() - 1).equals(target);
}
};
return foundInWindow.eval() || foundBefore.eval() || foundAfter.eval();
}
Полагаю, это действительно зависит от ситуации (как всегда). Для очень сложных условий подход обертки может быть защищен, хотя это редко.
Спасибо за ваш вклад!
РЕДАКТИРОВАТЬ: запоздалая мысль. Может быть даже лучше создать специальный класс для сложной логики, такой как этот:
import java.util.ArrayList;
import java.util.List;
public class IsTargetFoundExpression {
private final String target;
private final List<String> items;
private final int position;
private final int min;
private final int max;
public IsTargetFoundExpression(String target, List<String> items, int position, int min, int max) {
this.target = target;
this.items = new ArrayList(items);
this.position = position;
this.min = min;
this.max = max;
}
public boolean evaluate() {
return foundInWindow() || foundBefore() || foundAfter();
}
private boolean foundInWindow() {
return position >= min && position < max && (foundAtEvenPosition() || foundAtOddPosition());
}
private boolean foundAtEvenPosition() {
return position % 2 == 0 && items.get(position).equals(target);
}
private boolean foundAtOddPosition() {
return position % 2 == 1 && position > min && items.get(position - 1).equals(target);
}
private boolean foundBefore() {
return position < min && items.get(0).equals(target);
}
private boolean foundAfter() {
return position >= max && items.get(items.size() - 1).equals(target);
}
}
Логика достаточно сложна, чтобы гарантировать отдельный класс (и юнит-тест, ууу!). Это сделает код, использующий эту логику, более читабельным и способствует повторному использованию в случае, если эта логика нужна где-то еще. Я думаю, что это хороший класс, потому что он действительно несет единственную ответственность и только заключительные поля.