従属クラスと共有プロパティを持つ大クラス

Question

大規模なC＃クラスをリファクタリングする最適な方法を理解することができません。また、大きなクラスの共有プロパティ/値を抽出されたクラスに渡して、メインクラスでは

最初はこのクラスは1000行で、手続き型です。メソッドの呼び出しと特定のシーケンスでの作業が含まれます。途中で物事はデータベースに保存されます。プロセス中に、そのメソッドで作業され、共有される項目のリストが複数存在します。このプロセスの最後には、ユーザーに提示される一連の統計があります。これらの統計は、処理が行われているときにさまざまな方法で計算されます。大まかな概要を説明すると、プロセスにはランダムな選択が多数含まれており、プロセスの最後には、ランダムなアイテムの数、無効なレコードの数、サブリストの数などが表示されます。

私は、おじさんの「クリーンコード」を読んでいて、それぞれのクラスが1つしかないことを私がリファクタリングしていることを確認しようとしています。

私は、ファイルを小さく保つためにメソッドとクラスを抽出することができましたが（現在は450行まで減らしています）、私が今問題にしているのは、これらの分割されたクラスがメインの親クラスそれらに渡され更新されます - これらの値は他のメソッド/クラスメソッドにも使用されます。

たクリーンなアプローチされたように私が破れています：

1）私は、メインクラスに呼び出した後、メインクラスの統計値とリストを格納するためのプライベートメンバ変数の束を作成し、すべき複雑な結果クラスを受け取ってから、これらの値を抽出して、プライベートメンバー変数を設定/更新しますか？ （ボイラープレートコードの多くこうして）

OR

2）それはDTOまたはリストや統計値を保持するコンテナクラスのいくつかの並べ替えを作成し、ちょうど様々なクラスメソッドに渡した方が良いです値のリストを構築するために、参照によって子クラスのメソッドを使用していますか？言い換えれば、私はこのコンテナクラスを渡しているだけなので、オブジェクトであるため他のクラスとメソッドはそこの値を直接操作できます。そして、プロセスの終わりに、DTO/container /あなたがそれを呼び出すことを望むものはすべて最終的な結果を持ちます。そして、私はコンテナクラスからそれらを抽出できます。それらを抽出してメインクラスのプライベートメンバー変数を設定してください）。

これは私が今持っている方法ですが、これはコードの匂いであると感じています。私は、大規模なクラスは素晴らしいではありません知っているが、少なくとも1つの大きなファイル内のすべてで、それは

など、私が更新していたプロパティへと明確に見えるん - UPDATE -

いくつかの詳細情報：

残念ながら私はそれがpropriataryであるので、実際のコードを投稿することはできません - ダミーの例を思いついて、時間があれば貼り付けようとします。以下のコメントの1つは、コードをステップにリファクタリングすることであり、それはまさに私がやったことです。クラスの目的は、最終的に1つのことです。つまり、ランダムなリストを作成することです。このクラスのために呼び出される唯一のパブリックメソッドでは、これを各ステップの1段階の抽象レベルにリファクタリングしました。各ステップは、それが同じクラスのメソッドであっても、サブステップを実行するためのヘルパークラスに分解するのが理にかなっていても、プロセス中に構築されるリストや単純なカウンタ変数にアクセスする必要があります統計情報を記録しています。私は "周りのものを渡す" ではないでしょう

public class RandomList(){ 

    public int Id{get; set;} 
    public int Name{get; set;} 
    public int NumOfInvalidItems {get; set;} 
    public int NumOfFirstChunkItems{get; set;} 
    public int NumOfSecondChunkItems{get; set;} 

    public ICollection<RandomListItem> Items{get; set;} 
} 

public class CreateRandomListService(){ 

    private readonly IUnitOfWork _unitOfWork; 
    private readonly ICreateRandomListValidator _createRandomListValidator; 
    private readonly IRandomSubProcessService _randomSubProcessService; 
    private readonly IAnotherSubProcessService _anotherSubProcessService; 

    private RandomList _randomList; 

    public CreateRandomListService(IUnitOfWork unitOfWork, 
           ICreateRandomListValidator  createRandomListValidator, 
           IRandomFirstChunkFactory randomFirstChunkFactory, 
           IRandomSecondChunkFactory randomSecondChunkFactory){ 
    _unitOfWork = unitOfWork; 
    _createRandomListValidator = createRandomListValidator;  

    _randomFirstChunkService = randomFirstChunkFactory.Create(_unitOfWork); 
    _randomSecondChunkService = randomSecondChunkFactory.Create(_unitOfWork); 
} 


public CreateResult CreateRandomList(CreateRandomListValues createValues){ 

    // validate passed in model before proceeding 
    if(_createRandomListValidator.Validate(createValues)) 
     return new CreateResult({HasErrors:true}); 

    InitializeValues(createValues); // fetch settings from db etc and build up 
    ProcessFirstChunk(); 
    ProcessSecondChunk(); 
    SaveWithStatistics(); 
    createResult.Id = _randomList.Id; 
    return createResult; 


} 

private InitializeValues(CreateRandomListValues createValues){ 

    _createValues = createValues; 
    _createValues.ImportantSetting = _unitOfWork.SettingsRepository.GetImportantSetting(); 
    // etc. 
    _randomList = new RandomList(){ 
     // set initial properties etc. some come from the passed in createValues, some from db 
    } 

} 
private void ProcessFirstChunk(){ 
    _randomFirstChunkService.GetRandomFirstChunk(_createValues); 
} 

private void ProcessSecondChunk(){ 
    _randomSecondChunkService.GetRandomSecondChunk(_createValues); 
} 

private void SaveWithStatistics(){ 

    _randomList.Items _createValues.ListOfItems; 
    _randomList.NumOfInvalidItems = _createValues.NumOfInvalidItems; 
    _randomList.NumOfItemsChosen = _createValues.NumOfItemsChosen; 
    _randomList.NumOfFirstChunkItems = _createValues.NumOfFirstChunkItems; 
    _randomList.NumOfSecondChunkItems = _createValues.NumOfSecondChunkItems; 

    _unitOfWork.RandomThingRepository.Add(_randomList); 
    _unitOfWork.Save(); 
} 
} 

public class RandomFirstChunkService(){ 
    private IUnitOfWork _unitOfWork; 

    public RandomFirstChunkService(IUnitOfWork unitOfWork){ 
     _unitOfWork = unitOfWork; 
    } 

public void GetRandomFirstChunk(CreateRandomListValues createValues){ 
    // do processing here - build up list collection and keep track of counts 
    CallMethodThatUpdatesList(creatValues); 

    // how to return this to calling class? currently just updating values in createValues by reference 
    // can also return a complex class here and extract the values back to the main class' member 
    // variables 
} 

private void CallMethodThatUpdatesList(createRandomListValues createValues){ 
    // do work 

} 
} 

Answer 1

：

- - UPDATE

ここでは、コード内の似た何かを示すの試みです。 1000行しかないので、別のクラスに分けることもできません。あなたはそれをはるかに扱いにくくし、メンテナンスの頭痛をはるかにもたらすでしょう。

あなたはあなたのコード（duh）を投稿していないので、それを批評するのは難しいです。実際にそれを調べると、メソッドにリファクタリングできるコードが重複している可能性があります。

すでに重複しているコードを取り除いてしまった場合は、次にすべてのデータベースDALレイヤーに入れることができます。

あなたが提供した小さな情報に基づいて実際にそれを小さくしたい場合は、次にそれを「ステップ」にリファクタリングして、ワークフロータイプの親コンテナクラスを作成します。

また、コードを知らなくても難しいと言えます。

Answer 2

私はあなたがここまでクラスをリファクタリングするために管理しているかを正確に知りませんが、「統計」は、対象となるべき概念であるように、あなたの説明から、それは私に聞こえる、のようなもの：

interface IStatistic<TOutput> 
{ 
    IEnumerable<TOutput> Calculate(IEnumerable<input-type>); 
} 

統計オブジェクトを構築することは容易ではない場合には

return new MySpecial().Calculate(myData); 

、EI：あなたには、いくつかの統計を表示したい場合は

は、あなただけの適切な統計を使用します彼らはいくつかのパラメータを求めるので、あなたはそれらを作成したFuncデリゲートを供給することができる：

void DoSomething(Func<IStatistic<string>> factory) 
{ 
    string[] inputData = ... 
    foreach (string line in factory().Calculate(inputData)) 
    { 
     // do something... 
    } 
} 

あなたが複数のリストに言及しているように、私は、入力型が実際に入力タイプのカップルになると仮定します。それがそうであるならば、それは本当にただのリストを保持するためにDTOの種類を供給するのは意味があります：

class RawData 
{ 
    public IEnumerable<type1> Data1 { get; } 
    public IEnumerabel<type2> Data2 { get; } 
    ... 
} 

は「本を読んで」これはDTOはないこと、しかし、確認します。まず、それは不変です - ゲッターだけがそこにあります。次に、生のリストではなく、シーケンス（IEnumerable）のみを公開します。両方の措置は、統計オブジェクトがデータを操作できないようにするために行われます。

Answer 3

残酷な答えは、もちろん...それに依存するということです。コードを読まなくても解答するのは難しいですが、新しいクラスを作成したら（ある目的で）、それらのクラスとインターフェースは、あなたの問題を解決するために渡す必要のあるデータオブジェクトを定義する必要があります。そのような場合、メソッドと同じ型を返すメソッドが奇妙であり、メソッドの一連の操作を介して操作が壊れやすいとも考えています。あなたのクラスがそれぞれRESTサービスだったとしたらどうでしょうか？それらのインターフェースはどのように見えますか？