design-patterns - 長いオブジェクト呼び出しチェーンには本質的に問題がありますか?

Question

私は自分のコードを階層的に整理しており、次のようなコードを使用してツリーをクロールしていることに気付きました。

File clientFolder = task.getActionPlan().getClientFile().getClient().getDocumentsFolder();

taskオブジェクトにドリルダウンしているわけではありません。私はその親にドリルアップしているので、カプセル化に関して何も失っていないと思います。しかし、心の奥底で、このようにするのは何か汚いことだというフラグが立っています。

これは間違っていますか？

score 5 · Accepted Answer

旗は赤で、次の 2 つのことを太字で示しています。

チェーンをたどるには、呼び出し元のコードがツリー構造全体を知る必要があり、これは適切なカプセル化ではありません。
階層が変更された場合、編集するコードがたくさんあります

かっこ内の 1 つのこと:

task.getActionPlan() の代わりに task.ActionPlan などのプロパティを使用します。

より良い解決策は、すべての親プロパティをツリーの子レベルで公開する必要があると仮定して、先に進んで子に直接プロパティを実装することです。

File clientFolder = task.DocumentsFolder;

これにより、少なくとも呼び出し元のコードからツリー構造が隠されます。内部的には、プロパティは次のようになります。

class Task {
    public File DocumentsFolder {
        get { return ActionPlan.DocumentsFolder; }
    }
    ...
}
class ActionPlan {
    public File DocumentsFolder {
        get { return ClientFile.DocumentsFolder: }
    }
    ...
}
class ClientFile {
    public File DocumentsFolder {
        get { return Client.DocumentsFolder; }
    }
    ...
}
class Client {
    public File DocumentsFolder {
        get { return ...; } //whatever it really is
    }
    ...
}

ただし、ツリー構造が将来変更された場合、チェーンを呼び出したすべての場所ではなく、ツリーに含まれるクラスのアクセサー関数のみを変更する必要があります。

[さらに、上記の例では省略されていたプロパティ関数で null を適切にトラップして報告する方が簡単です]

score 4 · Accepted Answer

まあ、すべての間接化は、それがうまくいかない可能性のあるポイントを 1 つ追加します。

特に、このチェーンのメソッドのいずれかが null を返す可能性があります (理論的には、あなたのケースではそれを行うことができないメソッドがある可能性があります) 。どちらか。

したがって、メソッドのいずれかが null を返す可能性がある場合は、少なくともそれらのポイントでチェーンを分割し、中間変数に格納して個々の行に分割し、クラッシュレポートで見る行番号。

それとは別に、明らかな問題は見られません。null参照が問題にならないことを保証している、または保証できる場合、それはあなたが望むことをするでしょう。

読みやすさはどうですか？名前付き変数を追加すると、より明確になりますか? 常に、仲間のプログラマーに読み取られるようにコードを記述し、偶然にコンパイラーによって解釈されるだけにしてください。

この場合、一連のメソッド呼び出しを読んで理解する必要があります... OK、それはドキュメントを取得します、それはクライアントのドキュメントです、クライアントは...ファイルから来ています...そうです、そしてファイルはアクションプランなどからのものです。チェーンが長いと、たとえば次のように読みにくくなる可能性があります。

ActionPlan taskPlan = task.GetActionPlan();
ClientFile clientFileOfTaskPlan = taskPlan.GetClientFile();
Client clientOfTaskPlan = clientFileOfTaskPlan.GetClient();
File clientFolder = clientOfTaskPlan.getDocumentsFolder();

この件については、個人の見解に帰着すると思います。

score 3 · Accepted Answer

ゲッターとセッターは悪です。通常、オブジェクトに何かをさせることは避けてください。代わりに、タスク自体を委任します。

それ以外の

Object a = b.getA();
doSomething(a);

行う

b.doSomething();

すべての設計原則と同様に、盲目的にこれに従わないでください。getter と setter を使わずに非常に複雑なものを書くことはできませんでしたが、これは良いガイドラインです。ゲッターとセッターがたくさんある場合は、おそらく間違っていることを意味します。

score 2 · Accepted Answer

まず第一に、このようなコードのスタックは、NullPointerExceptions を分析し、デバッガーにステップインする際に参照をチェックするのを煩わしくする可能性があります。

それとは別に、私はそれがすべてこれに要約すると思います:発信者はそのすべての知識を持っている必要がありますか?

おそらく、その機能をより一般的なものにすることができます。代わりに File をパラメーターとして渡すことができます。それとも、ActionPlan は、その実装が ClientFile に基づいていることを明らかにすべきではないでしょうか?

score 1 · Accepted Answer

どのようにタイムリー。私は今夜私のブログにこの匂い、メッセージチェーンとその逆のミドルマンについて投稿するつもりです。

とにかく、より深い質問は、ドメインオブジェクトのように見えるものに「get」メソッドがある理由です。問題の輪郭に厳密に従うと、何かを取得するようにタスクに指示するのは意味がないか、UI表示の準備など、実際にはビジネスロジック以外の問題であることがわかります。永続性、オブジェクトの再構築など。

後者の場合、「get」メソッドは、許可されたクラスで使用されている限り問題ありません。そのポリシーをどのように実施するかは、プラットフォームとプロセスに依存します。

したがって、「get」メソッドが問題ないと見なされた場合でも、問題に直面する必要があります。そして残念ながら、それはチェーンをナビゲートしているクラスに依存すると思います。そのクラスを構造（たとえば、ファクトリ）に結合することが適切である場合は、それを許可します。それ以外の場合は、Delegateを非表示にしてみてください。

編集：私の投稿はここをクリックしてください。

score 1 · Accepted Answer

現実的に、これらの機能をすべて個別に使用するつもりですか? タスクに GetDocumentsFolder() メソッドを持たせて、これらすべてのメソッドを呼び出すという厄介な作業をすべて実行してみませんか? 次に、コードを作成する必要のない場所でコードを作成することなく、すべてをnullチェックするという汚い作業をすべて行うことができます。

score 1 · Accepted Answer

デメテルの法則に言及したポスターに同意します。あなたがしていることは、これらのクラスの多くの実装と、階層自体の構造に不要な依存関係を作成することです。テストしたいクラスの動作するインスタンスを取得するためだけに、多数の他のオブジェクトを初期化する必要があるため、コードを単独でテストすることは非常に困難になります。

score 0 · Accepted Answer

デメテルの法則も参考にさせていただきます。

そして、 Tell, Don't Askに関する記事を追加します。

score 0 · Accepted Answer

世界最大の旗。

これらの呼び出しのいずれかが null オブジェクトを返すかどうかを簡単に確認できないため、あらゆる種類のエラーを追跡することはほぼ不可能です!

getClientFile() は null を返す可能性があり、その後 getClient() は失敗します。これをキャッチすると、try キャッチを行っていると仮定すると、どれが失敗したかの手掛かりがありません。

score 0 · Accepted Answer

null または無効な結果が得られる可能性はどれくらいですか? そのコードは、多くの関数が正常に返されることに依存しており、null ポインター例外などのエラーを整理するのが難しくなる可能性があります。

デバッガーにとっても悪いことです。いくつかのローカル変数を監視するだけでなく、関数を実行する必要があるため、情報が少なく、チェーンの後の関数にステップインするのが面倒です。

score 0 · Accepted Answer

はい。それはベストプラクティスではありません。1 つには、バグがあると、それを見つけるのが難しくなります。たとえば、例外ハンドラーは、121 行目に NullReferenceException があることを示すスタックトレースを表示する場合がありますが、これらのメソッドのどれが null を返していますか? 調べるには、コードを掘り下げる必要があります。

score 0 · Accepted Answer

これは主観的な質問ですが、ある程度までは問題ないと思います。たとえば、チェーンがエディターの読み取り可能な領域を超えて拡張されている場合は、いくつかのローカルを導入する必要があります。たとえば、私のブラウザでは最後の 3 回の呼び出しが表示されないため、何をしているのかわかりません :)。

score 0 · Accepted Answer

まあそれは依存します。そのようなオブジェクトを介して到達する必要はありません。これらのメソッドの実装を制御する場合は、その必要がないようにリファクタリングすることをお勧めします。

そうでなければ、あなたがしていることに害はないと思います。よりも確かに優れている

ActionPlan AP = task.getActionPlan();
ClientFile CF = AP.getClientFile();
Client C = CF.getClient();
DocFolder DF = C.getDocumentsFolder();

score 0 · Accepted Answer

それ自体は悪くありませんが、6 か月でこれを読むのに問題が発生する可能性があります。または、オブジェクトのチェーンが非常に長いため、同僚がコードの保守/記述に問題を抱える可能性があります。

そして、コードに変数を導入する必要はないと思います。したがって、オブジェクトは、メソッドからメソッドへジャンプするために必要なすべてを知っています。（ここで、少しオーバーエンジニアリングしていないかという疑問が生じますが、誰に言えばいいですか？）

一種の「便利な方法」を紹介します。「タスク」にメソッドがあると想像してください-次のようなオブジェクト

    task.getClientFromActionPlan();

あなたは確かに使用することができます

    task.getClientFromActionPlan().getDocumentsFolder();

はるかに読みやすく、これらの「便利なメソッド」を正しく実行した場合（つまり、頻繁に使用されるオブジェクトチェーンの場合）、入力する必要ははるかに少なくなります;-) .

Edith は次のように述べています。このようにして、適切なエラーメッセージを含む Nullpointers をスローすることもできます (つまり、「クライアントからクライアントを取得しようとしているときに ActionPlan が null でした」)。

score 0 · Accepted Answer

0

関連する議論:

関数チェーン - 多すぎるのはいくつですか?

于 2008-10-01T20:49:10.980 に答える

score 0 · Accepted Answer

この質問はhttps://stackoverflow.com/questions/154864/function-chaining-how-many-is-too-many#155407に非常に近いです

一般に、連鎖が長すぎるのはよくないという意見が一致しているようで、連鎖呼び出しはせいぜい 1 つまたは 2 つに留めるべきです。

Python ファンは連鎖をとても楽しいと考えていると聞いていますが。それはただの噂かもしれません...:-)

score 0 · Accepted Answer

最終的な目標によっては、おそらく最小知識のプリンシパルを使用して、重度の結合と最終的なコストを回避することをお勧めします。頭が一番好きなように.. 「友達とだけ話してください」.

score 0 · Accepted Answer

チェーニングのもう 1 つの重要な副産物は、パフォーマンスです。

ほとんどの場合、これは大したことではありませんが、特にループでは、間接性を減らすことでパフォーマンスが大幅に向上することがわかります。

また、チェーンを使用すると、パフォーマンスの見積もりが難しくなります。これらのメソッドのどれが複雑なことを行うかどうかを判断することはできません。

score 0 · Accepted Answer

特定の階層にコードをロックしているため、他の人がコードが良くないと指摘しているのでOKです。デバッグで問題が発生する可能性があり、読みにくいですが、主なポイントは、タスクを実行するコードが、フォルダーを取得するためのトラバースについてあまりにも多くのことを知っていることです。ドーナツにドル、誰かが何かを真ん中に挿入したいと思うでしょう。（すべてのタスクはタスクリストなどにあります）

手足に出て、これらのクラスはすべて同じものの特別な名前ですか? つまり、それらは階層的ですが、各レベルにはおそらくいくつかの追加のプロパティがありますか?

したがって、別の角度から、子クラスが要求されたものでない場合、子クラスがチェーンを委任する列挙型とインターフェイスに単純化します。議論のために、私はそれらをフォルダーと呼んでいます。

enum FolderType { ActionPlan, ClientFile, Client, etc }

interface IFolder
{
    IFolder FindTypeViaParent( FolderType folderType )
}

そして IFolder を実装する各クラスはおそらくそうします

IFolder FindTypeViaParent( FolderType folderType )
{
    if( myFolderType == folderType )
        return this;

    if( parent == null )
        return null;

    IFolder parentIFolder = (IFolder)parent;
    return parentIFolder.FindTypeViaParent(folderType)
}

バリエーションは、IFolder インターフェイスを作成することです。

interface IFolder
{
    FolderType FolderType { get; }
    IFolder Parent { get; }
}

これにより、トラバーサルコードを外部化できます。ただし、これによりクラスから制御が奪われ (複数の親を持つ可能性があります)、実装が公開されます。良くも悪くも。

[とりとめのない]

一見すると、これは設定するのにかなりコストのかかる階層のように見えます。毎回トップダウンでインスタンス化する必要がありますか? つまり、タスクが必要なだけの場合、すべてのバックポインターが確実に機能するように、すべてをボトムアップでインスタンス化する必要がありますか? 遅延ロードであっても、ルートを取得するためだけに階層を上る必要がありますか?

もう一度言いますが、階層は本当にオブジェクトアイデンティティの一部なのでしょうか? そうでない場合は、階層を n-ary ツリーとして外部化できます。

補足として、集計の DDD (ドメイン駆動設計) の概念を検討し、主要なプレーヤーが誰であるかを判断することをお勧めします。責任を負う最終的な所有者オブジェクトは何ですか? たとえば、車の車輪。車をモデルにしたデザインでは、ホイールもオブジェクトですが、車が所有しています。

うまくいくかもしれませんが、うまくいかないかもしれません。私が言ったように、これは暗闇の中でのショットです。

design-patterns - 長いオブジェクト呼び出しチェーンには本質的に問題がありますか?

20 に答える 20

Related

Reference