2009-12-18 35 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做同樣的事情,但是用匿名類編寫代碼效率較低,而且效率較低。