2

時々、これらの行に沿ったコードで終わります。そこで、いくつかのオブジェクトを作成し、それらをループして、別のクラスを使用していくつかのプロパティを初期化します...

ThingRepository thingRepos      = new ThingRepository();
GizmoProcessor  gizmoProcessor  = new GizmoProcessor();
WidgetProcessor widgetProcessor = new WidgetProcessor();

public List<Thing> GetThings(DateTime date)
{
    List<Thing> allThings = thingRepos.FetchThings();

    // Loops through setting thing.Gizmo to a new Gizmo
    gizmoProcessor.AddGizmosToThings(allThings);

    // Loops through setting thing.Widget to a new Widget
    widgetProcessor.AddWidgetsToThings(allThings);

    return allThings;
}

...それは、まあ、間違っていると感じます。

  1. これは悪い考えですか？
  2. ここで使用しているアンチパターンの名前はありますか?
  3. 代替手段は何ですか？

編集:と の両方がオフになり、計算を実行し、他のテーブルから追加のデータを取得する必要があるGizmoProcessorと仮定します。WidgetProcessorそれらは、リポジトリに格納された単なるデータではありません。それぞれに基づいて新しいギズモとウィジェットを作成し、それらをのプロパティThingに割り当てています。Thing

これが私にとって奇妙に感じる理由は、それThingが自律的なオブジェクトではないからです。それ自体と子オブジェクトを作成することはできません。完全に完成したオブジェクトを作成するには、上位のコードが必要です。それが悪いことなのかどうかはわかりません！

4

4 に答える 4

1

ThingRepositoryのコレクションを取得するための単一のアクセス ポイントになるはずですThing。または、少なくとも開発者が直観的に調べる場所です。そのため、GetThings(DateTime date)別のオブジェクトで提供する必要があるのは奇妙に感じます。私はむしろそのメソッドThingRepository自体を配置したいと思います。

Thingによって返される がによって返されるGetThings(DateTime date)ものとは異なり、「太った」動物であるという事実も、ThingRepository.FetchThings()ぎこちなく直感に反するように感じます。GizmoとWidgetが実際にエンティティの一部である場合、 によって返されるインスタンスだけでなくThing、 のインスタンスがあるたびにそれらにアクセスできるはずです。ThingGetThings(DateTime date)

の Date パラメーターGetThings()が重要でない場合、または別の機会に収集できる場合は、 で計算されたプロパティを使用して、 およびThingへのオンデマンド アクセスを実装します。GizmoWidget

public class Thing
{
  //...

  public Gizmo Gizmo
  {
    get 
    { 
       // calculations here 
    }
  }

  public Widget Widget
  {
    get 
    { 
       // calculations here 
    }
  }
}

このアプローチは、実行される計算にコストがかかりすぎない限り有効であることに注意してください。高価な処理を伴う計算されたプロパティはお勧めしません。

ただし、これらの計算はゲッターでインラインで実装する必要はありません。キャッシング戦略などを使用して、サードパーティの Gizmo/Widget プロセッサに委任できます。

于 2013-01-15T10:23:08.350 に答える
0

複雑な初期化がある場合は、戦略パターンを使用できます。これは、この戦略パターンの概要から適応された簡単な概要です

初期化を抽象化する戦略インターフェースを作成する

public interface IThingInitializationStrategy
{
  void Initialize(Thing thing);
}

戦略で使用できる初期化の実装

public class GizmosInitialization
{
  public void Initialize(Thing thing)
  {
     // Add gizmos here and other initialization
  }
}

public class WidgetsInitialization
{
  public void Initialize(Thing thing)
  {
    // Add widgets here and other initialization
  }
}

最後に、抽象的な方法で戦略の実装を受け入れるサービス クラス

internal class ThingInitalizationService
{
  private readonly IThingInitializationStrategy _initStrategy;

  public ThingInitalizationService(IThingInitializationStrategy initStrategy)
  {
     _initStrategy = initStrategy;
  }

  public Initialize(Thing thing)
  {
    _initStrategy.Initialize(thing);
  }
}

その後、次のような初期化戦略を使用できます

var initializationStrategy = new GizmosInitializtion();
var initializationService = new ThingInitalizationService(initializationStrategy);


List<Thing> allThings = thingRepos.FetchThings();

allThings.Foreach ( thing => initializationService.Initialize(thing) );
于 2013-01-15T12:32:36.837 に答える
0

本当の潜在的な問題は、同じループを複数回繰り返していることだけですが、すべてのギズモとウィジェットを取得するためにデータベースにアクセスする必要がある場合は、それらをバッチで要求して完全なリストを渡す方が効率的かもしれませんAdd... メソッドに追加するのは理にかなっています。

他のオプションは、最初のリポジトリ呼び出しでギズモとウィジェットを返すことを検討することです (それらが同じリポジトリにあると仮定します)。クエリがより複雑になる可能性がありますが、おそらくより効率的です。もちろん、物事を取得するときに常にギズモやウィジェットを取得する必要があるわけではありません。

于 2013-01-15T00:47:43.117 に答える
0

質問に答えるには:

  1. これは悪い考えですか？

    • 私の経験から、変更する必要があるまで、それが良いアイデアか悪いアイデアかはほとんどわかりません。
    • IMO、コードは次のいずれかです: Over-engineered、under-engineered、またはunreadable
    • その間、最善を尽くし、ベスト プラクティス (KISS、単一の責任など) に固執します。
    • 個人的には、プロセッサ クラスがモノの状態を変更するべきではないと思います。
    • また、変更する対象のコレクションをプロセッサ クラスに与えるべきではないと思います。

  2. ここで使用しているアンチパターンの名前はありますか?

    • 申し訳ありませんが、お手伝いできません。

  3. 代替手段は何ですか？

個人的には、次のようにコードを記述します。

public List<Thing> GetThings(DateTime date)
{
    List<Thing> allThings = thingRepos.FetchThings();

    // Build the gizmo and widget for each thing
    foreach (var thing in allThings)
    {
        thing.Gizmo = gizmoProcessor.BuildGizmo(thing);
        thing.Widget = widgetProcessor.BuildWidget(thing);
    }

    return allThings;
}

私の理由は次のとおりです。

  1. コードは「ものを取得する」クラスにあります。論理的には、各 Thing オブジェクトをトラバースして初期化することは許容できると思います。
  2. 意図は明らかです。プロパティをThing返す前に、それぞれのプロパティを初期化しています。
  3. Thing中央の場所でプロパティを初期化することを好みます。
  4. gizmoProcessor クラスと widgetProcessor クラスは Collection of Things を扱うべきではないと思います
  5. プロセッサには、単一のウィジェット/ギズモを構築して返すメソッドがあることを好みます

ただし、プロセッサ クラスが一度に複数のプロパティを構築している場合は、プロパティの初期化を各プロセッサにリファクタリングするだけです。

public List<Thing> GetThings(DateTime date)
{
    List<Thing> allThings = thingRepos.FetchThings();

    // Build the gizmo and widget for each thing
    foreach (var thing in allThings)
    {
        // [Edited]
        // Notice a trend here: The common Initialize(Thing) interface
        // Could probably be refactored into some 
        // super-mega-complex Composite Builder-esque class should you ever want to
        gizmoProcessor.Initialize(thing);
        widgetProcessor.Initialize(thing);
    }

    return allThings;
}

追伸:

  • 個人的には（アンチ）パターンの名前はあまり気にしません。
  • より高いレベルの抽象化で問題を議論することは役に立ちますが、すべての (アンチ) パターン名をメモリにコミットすることはしません。
  • 役に立つと思うパターンに出くわすと、それを思い出すだけです。
  • 私は非常に怠け者であり、私の理論的根拠は次のとおりです。ほんの一握りしか使用しないのに、なぜわざわざすべてのパターンとアンチ パターンを覚える必要があるのでしょうか。

[編集]

Strategy Service の使用に関して既に回答が与えられていることに気付きました。

于 2013-01-16T11:01:35.477 に答える