2009-12-18 79 views
7

有时条件可能会变得相当复杂,所以为了便于阅读,我通常将它们拆分并给每个组件一个有意义的名称。然而,这会影响短路评估,这会造成问题。我想出了一个封装方法,但在我看来,它太冗长了。如何分解复杂的条件并保持短路评估?

任何人都可以想出一个整洁的解决方案呢?

为我的意思的例子参见下面的代码:

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方法实现的是逻辑保持本地。注意,即使是布尔表达式的声明也是本地的(我不认为我以前曾经遇到过本地类声明的用例)。

早期的回报:

早期恢复解决方案似乎足够了,即使我并不喜欢的习语。另一种表示法:

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; 
} 

我知道大多数人讨厌的标签和休息,而这种风格可能太罕见考虑可读性。

嵌套/分手,如果是:

它允许引进与优化相结合的评估有意义的名字。缺点是条件语句的复杂的树,可随之而来

摊牌

所以,看哪种方法我utlimately的青睐,我申请了几个建议的解决方案,以上面提出的复杂targetFound例子。下面是我的结果:

嵌套/分,如果的,有意义的名称 非常详细的,有意义的名称不会真正帮助这里的可读性

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重新包装 请注意,方法参数必须声明为最终 这种复杂的情况下冗长是可防御IMO 也许将封闭使这更容易,如果他们对(曾经同意 -

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); 
    } 
} 

的逻辑是足以保证一个单独的类复杂(单元测试,耶!)。它将使使用此逻辑的代码更具可读性,并在其他地方需要此逻辑的情况下促进重用。我认为这是一个很好的课,因为它真的有单一的责任,只有最后的领域。

+0

我喜欢你的第一个解决方案,但是通过适当的变量命名,它可能不是必需的。你的第二个解决方案太冗长的IMO。 – BacMan 2009-12-18 15:10:20

回答

6

我找换行和空白做了很好的工作,实际上是:

public static void main1(String[] args) { 

    if (args != null 
     && args.length == 2 
     && !args[0].equals(args[1]) 
     ) { 
      System.out.println("Args are ok"); 
    } 
} 

诚然这与我的(不受欢迎)的作品更好的支撑风格(上面没有显示),但即使在它上面如果您将闭幕式和开放式大括号放在他们自己的路线上(因此它们在最后一个条件结束时不会丢失),则可以正常工作。

我有时甚至评论的各个位:

public static void main1(String[] args) { 

    if (args != null    // Must have args 
     && args.length == 2   // Two of them, to be precise 
     && !args[0].equals(args[1]) // And they can't be the same 
     ) { 
      System.out.println("Args are ok"); 
    } 
} 

如果你真的想叫的东西出来,多if旨意去做:

public static void main1(String[] args) { 

    if (args != null) { 
     if (args.length == 2) { 
      if (!args[0].equals(args[1])) { 
       System.out.println("Args are ok"); 
      } 
     } 
    } 
} 

...任何优化的编译器会折叠。但对我而言,这可能有点太冗长了。

+0

多个if是最自然和最清晰的解决方案。它显示了究竟发生了什么。您也可以在每一行添加注释。但为什么你需要优化编译器?没有什么可以崩溃的。 – PauliL 2009-12-18 15:38:56

+0

每行的注释都有Eclipse格式化程序的优势,不会改变换行符。 – 2009-12-18 15:42:49

+0

深层嵌套在大多数情况下被认为是不好的做法。但我不确定这种特殊情况是否是例外。但它的确提高了可读性。 – BalusC 2009-12-18 15:45:00

6

如果您的目标是可读性,为什么不简单地打破线条并添加评论?

if (args != null    // args not null 
     && args.length == 2   // args length is OK 
     && !args[0].equals(args[1]) // args are not equal 
    ) { 

     System.out.println("Args are ok"); 
    } 
+0

你有点修改他的逻辑... – 2009-12-18 15:11:16

+0

我的不好。修正:-) – 2009-12-18 15:12:32

+0

我已修复剩余的位 – 2009-12-18 15:13:52

11

可以使用早返回(从一个方法),以达到相同的效果:

[施加一些修补]

public static boolean areArgsOk(String[] args) { 
    if(args == null) 
     return false; 

    if(args.length != 2) 
     return false; 

    if(args[0].equals(args[1])) 
     return false; 

    return true; 
    } 

    public static void main2(String[] args) { 

     boolean b = areArgsOk(args); 
     if(b) 
      System.out.println("Args are ok"); 
    } 
+0

您忘记将参数传递给areArgsOk()。 – BalusC 2009-12-18 15:18:20

+0

你说得对。谢谢。固定。 – 2009-12-18 15:20:57

+0

注意!他有&& - 条件。使用 if(args == null)返回false;如果(args.length!= 2)返回false; if(args [0] .equals(args [1]))返回false; 返回true; – r3zn1k 2009-12-18 15:21:06

0

拆分出来到一个单独的功能(提高可读性在主要())并添加评论(所以人们明白你想完成什么)

public static void main(String[] args) { 
    if (argumentsAreValid(args)) { 
      System.out.println("Args are ok"); 
    } 
} 


public static boolean argumentsAreValid(String[] args) { 
    // Must have 2 arguments (and the second can't be the same as the first) 
    return args == null || args.length == 2 || !args[0].equals(args[1]); 
} 

ETA:我也喜欢Itay在ArgumentsAreValid函数中使用早期返回的想法来提高可读性。

+0

感谢您的回复。我真的不明白用这种方法提取方法是如何改进的。添加评论是有用的。哦,是的,'布尔'应该是布尔值,并且方法名称在Java中以小写开头。 – 2009-12-20 12:24:30

+0

@Adriaan - 修复了那里的代码 - 现在太熟悉C#并且忘记了我的Java。就我个人而言,我认为将其分开使其更具可读性,但这当然是一种主观判断。 – 2009-12-21 13:08:50

-1

第一段代码真的没有错;你正在推翻一些东西,海事组织。

下面是另一种方式,虽然详细,但很容易理解。

static void usage() { 
    System.err.println("Usage: blah blah blah blah"); 
    System.exit(-1); 
} 

// ... 

if (args == null || args.length < 2) 
    usage(); 
if (args[0].equals(args[1])) 
    usage() 
+0

虽然它在安全管理器的存在下并不完全相同。 – 2009-12-18 16:05:07

+0

为了简洁,我的例子很简单。考虑一个非常复杂的情况。上述两个条件语句的分割对我来说似乎是任意的。 – 2009-12-20 12:26:39

+0

是的,当然是任意的!你编写代码供人阅读,因此你可以对最清楚的内容作出判断。 – 2009-12-20 14:48:10

2

BooleanExpression类型看起来并没有足够的用处,可以在您的主类之外成为一个类,它也为您的应用程序增加了一些智力。我只需编写适当命名的私有方法即可运行所需的检查。它更简单。

+0

我认为创建私有方法仍然太侵入(并且需要传递很多参数)。 BooleanExpression在方法范围内,所以我可以分割我的条件而不会混乱我的顶级类。 – 2009-12-22 10:30:05

+0

我不同意。使用私有方法比将类嵌入方法更容易理解。实际上,BooleanExpression的定义方式,你也可以使用私有方法。不仅如此,而且因为它们是私密的,它们可以稍后改变。 如果这是Groovy或Python这样的语言,那么我会说通过一个方法(或闭包)来处理验证。既然不是这样,最好让事情变得简单(需要支持的人会感谢你)。 BooleanExpression是模仿闭包(或方法参数)的尝试。 – 2009-12-22 14:58:22

2

您必须执行变量赋值INSIDE if。

if (a && b && c) ... 

转化为

calculatedA = a; 
if (calculatedA) { 
    calculatedB = b; 
    if (calculatedB) { 
    calculatedC = c; 
    if (calculatedC) .... 
    } 
} 

它往往是有益无论如何要做到这一点,因为它的名字你正在测试的概念,正如你在示例代码清楚地表明。这增加了可读性。

1

您的第一个解决方案适合这种复杂性。如果条件更复杂,我会为每个需要运行的检查编写私有方法。像这样:

public class DemoStackOverflow { 

    public static void main(String[] args) { 
    if (areValid(args)) { 
     System.out.println("Arguments OK"); 
    } 
    } 

    /** 
    * Validation of parameters. 
    * 
    * @param args an array with the parameters to validate. 
    * @return true if the arguments are not null, the quantity of arguments match 
    * the expected quantity and the first and second are not equal; 
    *   false, otherwise. 
    */ 
    private static boolean areValid(String[] args) { 
     return notNull(args) && lengthOK(args) && areDifferent(args); 
    } 

    private static boolean notNull(String[] args) { 
     return args != null; 
    } 

    private static boolean lengthOK(String[] args) { 
     return args.length == EXPECTED_ARGS; 
    } 

    private static boolean areDifferent(String[] args) { 
     return !args[0].equals(args[1]); 
    } 

    /** Quantity of expected arguments */ 
    private static final int EXPECTED_ARGS = 2; 

} 
+0

这是我建议的答案的插图。 +1 – 2009-12-18 16:16:40

0

已经有一些很好的解决方案,但这是我的变化。我通常将空检查作为所有(相关)方法中最重要的守卫条款。如果我在这种情况下这样做,那么它在后续中只留下长度和相等性检查,这可能已经被认为充分降低了复杂性和/或可读性的提高?

public static void main1(String[] args) { 

     if (args == null) return; 

     if (args.length == 2 && !args[0].equals(args[1])) { 
      System.out.println("Args are ok"); 
     } 
    } 
+0

务实,但对于我认为的复杂案例还不够。 – 2009-12-22 10:30:41

0

您的第一个解决方案在许多情况下都不起作用,包括上面给出的示例。如果参数为空,则

boolean argsNotNull = args != null; 
// argsNotNull==false, okay 
boolean argsLengthOk = args.length == 2; 
// blam! null pointer exception 

短路的一个优点是它可以节省运行时间。另一个优点是它允许您进行早期测试,检查是否会导致稍后测试引发异常的条件。个人而言,当测试单独简单并且所有这些都使得它变得复杂时,它们中有很多,我会投票给简单的“添加换行符和注释”解决方案。这比创建一堆附加功能更容易阅读。

我把事情分解成子程序的唯一时候是个别测试很复杂。如果你需要走出去,读取数据库或执行大运算,然后确定,卷到这一个子程序这样的顶级代码是一个易于阅读的

if (salesType=='A' && isValidCustomerForSalesTypeA(customerid)) 
etc 

编辑: 我怎么会打破你给出的更复杂的例子。

当我真的得到这个复杂的条件时,我试着将它们分解为嵌套的IFs,以使它们更具可读性。就像......并且对不起,如果下面的内容与你的例子不一样,我不想过于紧密地研究括号,只是为了这样一个例子(当然括号也是很难读的) :

if (position < min) 
{ 
    return (items.get(0).equals(target)); 
} 
else if (position >= max) 
{ 
    return (items.get(items.size() - 1).equals(target)); 
} 
else // position >=min && < max 
{ 
    if (position % 2 == 0) 
    { 
    return items.get(position).equals(target); 
    } 
    else // position % 2 == 1 
    { 
    return position > min && items.get(position - 1).equals(target); 
    } 
} 

这似乎对我来说是可读的,因为它很可能会得到。在这个例子中,“顶级”条件显然是位置与最小和最大的关系,所以在我看来,打破这一点确实有助于澄清事物。

实际上,上述方法可能比将所有内容填充在一行上效率更高,因为else允许您减少比较次数。

就我个人而言,如果把一个复杂的条件放到一行中是一个好主意,那么我会奋斗。但即使你明白这一点,可能出现的情况是下一个出现的人不会。即使是一些相当简单的事情,如

return s == null? -1:s.length();

有时我告诉自己,是的,我明白了,但别人不会的,也许更好的写

if (s==null) 
    return -1; 
    else 
    return s.length(); 
+0

第一个解决方案打破短路,这是我的观点。我不考虑大型计算或从数据库中读取数据,只是考虑到所有数据都存在这个复杂的条件。 – 2009-12-20 12:33:13

+0

@Adriaan:好的。我的印象是,你认为这只是一个有效的东西,而不是“不起作用”的东西。好吧,我想我的回答是正确的,即使你已经知道了! – Jay 2009-12-21 14:20:17

+0

好的,那么你会如何展现我在顶部添加的更复杂的示例条件? – 2009-12-21 22:16:15

1

这个问题是从2009年,但未来(的Java 8),我们将能够使用Lambda表达式可能就像布尔表达式一样可用于此上下文,但您可以使用它,以便只在需要时对其进行评估。

public static void main2(String[] args) { 

    Callable<Boolean> argsNotNull =() -> args != null; 
    Callable<Boolean> argsLengthOk =() -> args.length == 2; 
    Callable<Boolean> argsAreNotEqual =() -> !args[0].equals(args[1]); 

    if (argsNotNull.call() && argsLengthOk.call() && argsAreNotEqual.call()) { 
     System.out.println("Args are ok"); 
    } 
} 

你可以用java 5/6做同样的事情,但是用匿名类编写代码效率较低,而且效率较低。