0

既存のクラスの再設計に取り組んでいます。このクラスでは、ほとんどの作業を行う約400行のwhileループです。ループの本体は、ifステートメント、変数割り当ての地雷原であり、中央のどこかに「続行」があります。ループの目的を理解するのは難しいです。

擬似コードでは、これが私が再設計しているところです：

/* Some code here to create the objects based on config parameters   */
/* Rather than having if statements scattered through the loop I     */
/* create instances of the appropriate classes.  The constructors     */
/* take a database connection.                                       */

FOR EACH row IN mySourceOfData
  int p = batcher.FindOrCreateBatch( row );
  int s = supplierBatchEntryCreator.CreateOrUpdate( row, p );
  int b = buyerBatchEntryCreator.CreateOrUpdate( row, p );
  mySouceOfData.UpdateAsIncludedInBatch( p, s, b);
NEXT
/* Allow things to complete their last item */
mySupplierBatchEntry.finish();
myBuyerBatchEntry.finish();
myBatcher.finish();

/* Some code here to dispose of things */

RETURN myBatch.listOfBatches();

FindOrCreateBatch（）内で、新しいバッチを作成する必要があるかどうか、または既存のバッチを使用できるかどうかを、いくつかのルールを使用して判断します。このインターフェースの実装が異なれば、それらを見つける方法などについても異なるルールがあります。戻り値は、見つけた、または作成した支払いバッチのデータベースからの代理キー（id）です。このIDは、pをパラメーターとして受け取る次のプロセスで必要になります。

これは私が始めたところからの改善ですが、このループを含むクラスに不安を感じています。

  1. ドメインオブジェクトではなく、「Manager」または「Controller」タイプのクラスのようです。
  2. バッチャーとsupplierBatchEntryCreator（および他のクラス）の間に入っているようです。現時点ではintのみが渡されますが、それが変更された場合は、3つのクラスすべてを変更する必要があります。これは、認知症の法則違反のようです。

何か提案がありますか、それとも大丈夫ですか？実際の言語はjavaです。

4

3 に答える 3

5

私はあなたに尋ねるいくつかの質問があります：

  • それは機能しますか？
  • 十分速いですか？
  • 読み取り可能/保守可能ですか？

3つすべての答えが「はい」の場合、それを超えて、それ以上の変更は、私の意見では本当に無駄な努力です。リファクタリングのためだけにリファクタリングしないでください。

あまりにも頻繁に、人々は何が起こるかを見越して物事を変更します（たとえば、あなたの「変化するint」）。私はYAGNIの思想の学校に加入することを好みます。それを心配する適切な時期はあなたがそれをするときです。

そして、デメテルの法則は設計ガイドラインであり、規則ではありません。現実の世界では、プラグマティズムは通常、ドグマティズムを打ち負かします:-)

于 2009-08-13T00:56:14.957 に答える
0

各XXXEntryCreatorとXXXEntryの関係は何ですか？「Creators」は整数しか返さないので、何かが足りないような気がします。

それを超えて、画面に収まるものに400行のクラッドを取り、ステップ間に適度に目に見えるデータフローがあります。称賛。（私は過去にそのような変更を試みたことに対して強い抵抗を経験しました-なぜ人々はN-100 / 1000ラインランオンを書くのですか？

于 2009-08-13T00:59:45.503 に答える
0

FindOrCreate複数のパスがより簡単になる可能性があることを私にCreateOrUpdate提案します（コードの残りの部分がわからないため、パフォーマンスが低下するかどうかはわかりません。これは、複数のパスが提案されたときに発生する一般的な懸念事項です）。

欠落しているバッチ、サプライヤー、およびバイヤーを作成するループが 1 つ (または 3 つのループ) ある場合、このループは次のように削減できます。

FOR EACH row IN mySourceOfData
  int p = batcher.FindBatch( row );
  int s = supplierBatchEntryCreator.Update( row, p );
  int b = buyerBatchEntryCreator.Update( row, p );
  mySouceOfData.UpdateAsIncludedInBatch( p, s, b);
NEXT

クリエーターが更新しているように見えますが、そうですか? おそらく、作成と更新の責任を 2 つのクラスに分割することは理にかなっているでしょうか?

私には少しシンプルに見え始めています。それは役に立ちますか？

于 2009-08-13T17:00:58.920 に答える