6

アクセサー内で例外をスローすることの賛否両論に関するいくつかの回答を読みましたが、例を挙げて特定の質問に答えると思いました。

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ます。この状況は、メソッドによってより適切に処理されますか？

4

8 に答える 8

7

はい、それは悪いです。次のように、宣言で文字列変数を初期化することをお勧めします。

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

セッターを次のようなものに置き換えることができます

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

したがって、NPEの可能性はありません。

（または、宣言で文字列変数を初期化せず、初期化をaddToListメソッドに追加し、それを使用するコードでgetStrings（）をチェックしてnullが返されるかどうかを確認します。）

なぜそれが悪いのかというと、そのような不自然な例では言うのは難しいですが、状態の変化が多すぎるようで、ユーザーに例外を処理させることで、このクラスのユーザーにペナルティを課しています。また、プログラム内のメソッドがExceptionをスローし始めると、コードベース全体に転移する傾向があるという問題もあります。

このインスタンスメンバーが設定されていることを確認する必要があるのはどこかですが、このクラスよりも何が起こっているのかについての知識が豊富な場所で行う必要があると思います。

于 2011-09-07T13:07:42.457 に答える
7

それは本当にあなたが得るリストで何をしているのかに依存します。Colletions.EMPTY_LISTより洗練された解決策は、または、@ Nathanが示唆するように、空のを返すことだと思いますArrayList。次に、リストを使用するコードは、必要に応じてリストが空であるかどうかを確認したり、他のリストと同じようにリストを操作したりできます。

違いは、文字列がないことと文字列のリストがないことの違いです。1つ目は空のリストで表し、2つ目nullは例外を返すかスローすることで表す必要があります。

于 2011-09-07T13:08:16.840 に答える
6

ゲッターを呼び出す前にリストを他の場所に設定する必要があると仮定して、これを処理する正しい方法は次のとおりです。

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

    return strings;
}

未チェックの例外であるため、メソッドで宣言する必要はありません。そのため、多くのtry/catchノイズでクライアントコードを汚染することはありません。また、この状態は回避可能であるため（つまり、クライアントコードはリストが初期化されていることを確認できます）、プログラミングエラーであり、したがって、チェックされていない例外は許容され、望ましいものです。

于 2011-09-07T13:09:59.763 に答える
2

一般的に、これはデザインの匂いです。

文字列が初期化されていないことが本当にエラーである場合は、セッターを削除してコンストラクターで制約を適用するか、ボヘミアンが言うように実行して、説明メッセージとともにIllegalArgumentExceptionをスローします。前者を実行すると、フェイルファスト動作が発生します。

文字列がnullの場合はエラーではない場合は、リストを空のリストに初期化することを検討し、上記と同様に、セッターでnull以外になるように強制します。これには、クライアントコードでnullをチェックする必要がないという利点があります。

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

これにより、クライアントコードを大幅に簡素化できます。

編集：わかりました。JSONからシリアル化しているのを見たばかりなので、コンストラクターを削除することはできません。したがって、次のいずれかを行う必要があります。

  1. ゲッターからIllegalArgumentExceptionをスローします
  2. 文字列がnullの場合、Collections.EMPTY_LISTを返します。
  3. 以上：逆シリアル化の直後に検証チェックを追加して、文字列の値を具体的にチェックし、適切な例外をスローします。
于 2011-09-07T13:16:55.120 に答える
1

元の質問の背後にあるより重要な質問を公開したと思います。ゲッターとセッターの例外的なケースを防ぐために作業する必要がありますか？

答えはイエスです、あなたは些細な例外を避けるために働くべきです。

ここにあるのは、この例ではまったく動機付けられていない、事実上怠惰なインスタンス化です。

コンピュータプログラミングでは、レイジー初期化は、オブジェクトの作成、値の計算、またはその他の高価なプロセスを、最初に必要になるまで遅らせる戦術です。

この例では、次の2つの問題があります。

  1. あなたの例はスレッドセーフではありません。nullのチェックは、2つのスレッドで同時に成功する（つまり、オブジェクトがnullであることがわかる）可能性があります。次に、インスタンス化により、2つの異なる文字列リストが作成されます。非決定論的な振る舞いが続きます。

  2. この例では、インスタンス化を延期する正当な理由はありません。費用がかかる、または複雑な操作ではありません。これは、「些細な例外を回避するための作業」の意味です。サイクルを投資して、有用な（ただし空の）リストを作成し、遅延爆轟nullポインター例外をスローしないようにする価値があります。

外部コードに例外を課すときは、基本的に、開発者がそれを使って賢明なことを行う方法を知っていることを望んでいることを忘れないでください。また、あなたのような例外をキャッチして無視するためだけに、すべてを例外イーターでラップした方程式に3番目の開発者がいないことを望んでいます。

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

以下の例では、Null Objectパターンを使用して、例が設定されていないことを将来のコードに示しています。ただし、Null Objectは例外的ではないコードとして実行されるため、将来の開発者のワークフローにオーバーヘッドや影響を与えることはありません。

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.
    }
}
于 2011-09-07T13:35:45.807 に答える
0

これについては、 JavaBeansの仕様を参照してください。おそらく、java.lang.Exceptionをスローせずに、代わりにjava.beans.PropertyVetoExceptionを使用し、Bean自体をjava.beans.VetoableChangeListenerとして登録して、適切なイベントを発生させる方がよいでしょう。それは少しやり過ぎですが、あなたがやろうとしていることを正しく識別します。

于 2011-09-07T13:11:43.007 に答える
0

それは本当にあなたがやろうとしていることに依存します。ゲッターが呼び出される前に設定が呼び出されたという条件を強制しようとすると、IllegalStateExceptionがスローされる場合があります。

于 2011-09-07T13:13:44.783 に答える
-1

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によっては、コンストラクターから直接渡すことはできませんか？そうすれば、セッターメソッドさえ必要ないでしょう。

于 2011-09-07T13:06:23.910 に答える