3

最近、副作用のある方法は良くないという記事を読んだことがあります。ですから、ここでの私の実装が副作用があると分類できるかどうかを尋ねたいと思います。

SecurityGuard顧客がクラブに行くことを許可する必要があるかどうかを確認するaがあるとします。

SecurityGuardどちらにもvalidNamesのリストまたはinvalidNamesのリストのみがあり、両方はありません。

  • SecurityGuardにvalidNamesしかない場合、彼はリストに名前が載っている顧客のみを許可します。
  • SecurityGuardにinvalidNamesしかない場合、彼は名前がリストにない顧客のみを許可します。
  • SecurityGuardリストがまったくない場合、彼はすべての人を許可します。

したがって、ロジックを適用するために、各リストのセッターで、新しいリストに値がある場合は、他のリストをリセットします。

class SecurityGaurd {
    private List<String> validNames = new ArrayList<>();
    private List<String> invalidNames = new ArrayList<>();

    public void setValidNames(List<String> newValidNames) {
        this.validNames = new ArrayList<>(newValidNames);
        // empty the invalidNames if newValidNames has values
        if (!this.validNames.isEmpty()) {
            this.invalidNames = new ArrayList<>();
        }
    }

    public void setInvalidNames(List<String> newInvalidNames) {
        this.invalidNames = new ArrayList<>(newInvalidNames);
        // empty the validNames if newInvalidNames has values
        if (!this.invalidNames.isEmpty()) {
            this.validNames = new ArrayList<>(); //empty the validNames
        }
    }


    public boolean allowCustomerToPass(String customerName) {
        if (!validNames.isEmpty()) {
            return validNames.contains(customerName);
        }
        return !invalidNames.contains(customerName);
    }
}

したがって、ここでは、setterメソッドに暗黙のアクションがあり、他のリストがリセットされていることがわかります。

問題は、私がここで行っていることは、副作用があると見なされる可能性があるということです。私たちがそれを変えなければならないほどそれは十分に悪いですか？はいの場合、どうすればこれを改善できますか？

前もって感謝します。

4

6 に答える 6

6

まあ、セッター自体には副作用があります (そのインスタンスの値は、関数の終了後も変更されたままになります)。だから、いいえ、私はそれを変更する必要がある悪いことだとは考えていません.

于 2013-03-01T04:44:09.497 に答える
3

ガードが、定義済みSetAdmissionPolicyへの参照を受け入れるものを持っていると想像してください。AdmissionPolicy

interface AdmissionPolicy {
    boolean isAcceptable(String customerName) {
}

ガードのadmissionPolicyフィールドを渡された参照に設定します。allowCustomerToPass単に と呼ばれるガード独自のメソッドadmissionPolicy.isAcceptable(customerName);。

上記の定義を考えると、 を実装する 3 つのクラスを想像できますAdmissionPolicy。1 つはコンストラクターでリストを受け入れ、リストisAcceptable上のすべてのユーザーに対して true を返し、別のクラスはコンストラクターでリストも受け入れますが、isAcceptable人に対してのみ true を返します。リストにありません。3 分の 1 は無条件に true を返します。クラブを時々閉鎖する必要がある場合は、無条件に false を返す 4 つ目の実装もあるかもしれません。

このように表示され、両方とも次のようsetInvalidNamesにsetValidNames実装できます。

public void setAdmissionPolicyAdmitOnly(List<String> newValidNames) {
    admissionPolicy = new AdmitOnlyPolicy(newValidNames);
}

public void setAdmissionPolicyAdmitAllBut(List<String> newInvalidNames) {
    admissionPolicy = new AdmitAllButPolicy(newInvalidNames);
}

このような実装では、各メソッドが 1 つのことだけを「設定」していたことは明らかです。そのような実装は、あなたのようなクラスがどのように振る舞うと私が期待するかです。

しかし、説明されているクラスの動作は、せいぜい疑わしいと思います。問題は、許可されたアイテムを追加すると拒否されたアイテムが消去されるということではなく、渡されたリストが空の場合の動作が、かなり奇妙な方法で以前の状態に依存することです。Fred 以外の全員がアクセスを許可されている場合、setValidNames を呼び出しても何の効果もないはずですが、George のみにアクセスを許可するように設定されている場合は、同じ呼び出しで全員にアクセスが許可されるはずです。さらに、有効な名前のリストに含まれていた人をsetValidNames削除するinvalidNamesことも、その逆も予想外ではありませんが、関数の命名方法を考えると、1 つのリストを設定するとすべての人が削除されるという事実他のリストからはやや予想外です（空のリストの動作が異なるため、特にそうなります）。

于 2013-12-29T20:03:27.780 に答える
1

副作用はありませんが、開発者は、ゲッターとセッターが変数の取得と設定以外に基になるコードを持たない可能性があると想定しています。したがって、別の開発者がコードを維持しようとすると、おそらく Bean のコードを見落とし、セッターで行ったのと同じチェックを行うでしょう - あなたが呼ぶボイラープレートコードの可能性

于 2013-03-01T04:52:11.257 に答える
0

私はそれを副作用とは考えていません。オブジェクトの根底にある仮定を維持しています。それが最高のデザインかどうかはわかりませんが、確かに機能するものです。

于 2013-03-01T04:44:32.883 に答える
0

この場合、スコープはこのクラス内にあるため、他のリンクリストを変更しても副作用はないと思います。

ただし、あなたの説明に基づいて、ホワイトリストとブラックリストを区別する1つのlinkedList（nameListと呼ばれる）とブール値（isValid）を持つ方が良いかもしれません。このようにして、常に 1 つのタイプのリストのみが埋められることは明らかです。

于 2013-03-01T04:52:22.670 に答える
0

大丈夫だと思います。たとえば、クラスを不変にしたい場合、それを行うのに最適な場所はセッターです:

public void setNames(List<String> names) {
       this.names = names == null ? Collections.emptyList() : Collections.unmodifiableList(names);
}
于 2013-03-01T05:04:58.483 に答える