糟糕的設計決定從存取器中拋出異常？

Question

我已閱讀再親的和訪問中拋出異常的反對的一些答案，但我想我會戳破我的具體問題用一個例子：

public class App { 
    static class Test {  
    private List<String> strings; 

    public Test() { 
    } 

    public List<String> getStrings() throws Exception { 
     if (this.strings == null) 
      throw new Exception(); 

     return strings; 
    } 

    public void setStrings(List<String> strings) { 
     this.strings = strings; 
    } 
} 

public static void main(String[] args) { 
    Test t = new Test(); 

    List<String> s = null; 
    try { 
     s = t.getStrings(); 
    } catch (Exception e) { 
     // TODO: do something more specific 
    } 
    } 
} 

getStrings()被拋出Exception時strings一直沒有組。這種情況能通過一種方法更好地處理嗎？

Answer 1

是的，那很糟糕。我建議初始化字符串變量聲明，如：

private List<String> strings = new ArrayList<String>(); 

，你可以用一些替代的setter像

public void addToList(String s) { 
    strings.add(s); 
} 

所以沒有一個NPE的可能性。

（或者沒有初始化的變量聲明字符串，初始化添加到addToList方法，以及使用它檢查getStrings（），看它是否得到空回有代碼。）

至於爲什麼它很糟糕，很難用這樣一個人爲的例子來說，但它似乎有太多的改變狀態，並且通過讓用戶處理異常來懲罰這個類的用戶。還有一個問題就是，一旦程序中的方法開始拋出異常，那麼它往往會轉移到整個代碼庫。

檢查這個實例成員是否被填充需要在某個地方完成，但是我認爲它應該在比這個類更清楚發生什麼的地方完成。

Answer 2

我建議拋出setter方法的異常。有沒有什麼特別的理由爲什麼不這樣做更好？

public List<String> getStrings() throws Exception { 
    return strings; 
} 

public void setStrings(List<String> strings) { 
    if (strings == null) 
     throw new Exception(); 

    this.strings = strings; 
} 

此外，根據特定的語境，那豈不是可以簡單地直接由構造函數傳遞List<String> strings？這樣，也許你甚至不需要setter方法。

Answer 3

這真的取決於你得到的列表正在做什麼。我認爲一個更優雅的解決方案是返回一個Colletions.EMPTY_LIST，或者，如@Nathan所示，一個空的ArrayList。然後，使用該列表的代碼可以檢查它是否爲空，如果它想要，或只是與其他列表一樣使用它。

區別在於之間沒有字符串和之間的區別沒有字符串列表。第一個應該由空列表表示，第二個應該返回null或拋出異常。

Answer 4

來處理這個正確的方法，假設它是名單中的其他地方設置之前調用吸氣的要求，如下：

public List<String> getStrings() { 
    if (strings == null) 
     throw new IllegalStateException("strings has not been initialized"); 

    return strings; 
} 

作爲一個選中例外，它不是」 t要求你在方法上聲明它，所以你不會用很多try/catch噪聲污染客戶端代碼。另外，由於這種情況是可以避免的（即客戶端代碼可以確保列表已被初始化），所以這是一個編程錯誤，因此未經檢查的異常是可以接受的並且是可取的。

Answer 5

我想諮詢一下Java Beans規範。可能最好不要拋出java.lang.Exception，而是使用java.beans.PropertyVetoException，讓bean將自己註冊爲java.beans.VetoableChangeListener並觸發適當的事件。這有點矯枉過正，但會正確識別你正在嘗試做什麼。

Answer 6

這真的取決於你想要做什麼。如果您試圖強制調用getter之前調用該設置的條件，則可能會出現拋出IllegalStateException的情況。

Answer 7

一般來說，這是一種設計氣味。

如果確實是字符串未初始化的錯誤，那麼要麼刪除setter，並在構造函數中強制約束，要麼像Bohemian所說的那樣，並用解釋性消息拋出IllegalArgumentException。如果你做前者，你會得到失敗的行爲。

如果不是錯誤，則字符串爲空，則考慮將列表初始化爲空列表，並在setter中將其強制爲非空，與上面類似。這具有以下優點：您無需在任何客戶端代碼中檢查null。

for (String s: t.getStrings()) { 
    // no need to check for null 
} 

這可以簡化客戶端代碼很多。

編輯：好的，我剛纔看到你是從JSON序列化，所以你不能刪除構造函數。所以，你需要：

1. 從吸氣
2. 拋出IllegalArgumentException如果字符串爲空，返回Collections.EMPTY_LIST。
3. 或更好：在反序列化之後添加一個驗證檢查，您可以專門檢查字符串的值，並引發一個明顯的異常。

Answer 8

我認爲你已經暴露了一個更重要的問題背後的原始問題：你應該努力防止在getters和setter的特例？

答案是肯定的，你應該努力避免微不足道的例外。

你這裏有什麼有效的lazy instantiation未在這個例子中，在所有的動機：

在計算機編程，懶惰初始化延遲 創建一個對象，值的計算策略，或其他一些其他 昂貴的過程，直到第一次需要。

你必須在這個例子中的兩個問題：

1. 你的例子不是線程安全的。該檢查null可以同時在兩個線程上成功（即，發現該對象爲空）。然後您的實例化將創建兩個不同的字符串列表。非確定性行爲將隨之而來。

2. 在這個例子中沒有很好的理由推遲實例化。這不是一個昂貴或複雜的操作。這就是我所說的「避免瑣碎的例外」：值得投入週期來創建一個有用的（但是空的）列表，以確保你不會拋出延遲爆炸空指針異常。

記住，當你對造成外部的代碼異常，你基本上希望開發人員知道如何理智做事吧。你也希望不存在，其在異常食裹一切方程在第三開發者，正好趕上並忽略像您一樣的例外：

try { 
    // I don't understand why this throws an exception. Ignore. 
    t.getStrings(); 
} catch (Exception e) { 
    // Ignore and hope things are fine. 
} 

在下面的例子中，我使用Null Object pattern向未來的代碼表明您的示例尚未設置。但是，Null對象作爲非常規代碼運行，因此不會對未來開發人員的工作流程產生開銷和影響。

public class App { 
    static class Test {    
     // Empty list 
     public static final List<String> NULL_OBJECT = new ArrayList<String>(); 

     private List<String> strings = NULL_OBJECT; 

     public synchronized List<String> getStrings() { 
      return strings; 
     }  

     public synchronized void setStrings(List<String> strings) {   
      this.strings = strings;  
     } 
    } 

    public static void main(String[] args) {  
     Test t = new Test();  

     List<String> s = t.getStrings(); 

     if (s == Test.NULL_OBJECT) { 
      // Do whatever is appropriate without worrying about exception 
      // handling. 

      // A possible example below: 
      s = new ArrayList<String>(); 
      t.setStrings(s); 
     } 

     // At this point, s is a useful reference and 
     // t is in a consistent state. 
    } 
}