糟糕的设计决定从存取器中抛出异常？

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