代码之家 › 专栏 › 技术社区 › WW.

帮助我重构此循环

law-of-demeter refactoring

0

WW. · 技术社区 · 15 年前

我正在重新设计一个现有的类。在这个类中,大约有一个400行的while循环可以完成大部分工作。循环体是一个包含if语句、变量赋值的雷区,中间有一个“continue”。循环的目的很难理解。

在伪代码中,这里是我重新设计的地方:

/* 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)。以下以p为参数的过程需要此ID。

这是对我开始的地方的改进,但是我对包含这个循环的类有一种不安的感觉。

它似乎不是一个域对象,而是一个“管理器”或“控制器”类型的类。
它似乎正在进入批处理程序和供应商批处理入口创建者(以及其他类)之间。目前只传递了一个int,但是如果这改变了所有三个类,那么需要改变。这似乎是违反逐出中心法。

有什么建议吗,还是可以?实际的语言是Java。

3 回复 | 直到 15 年前

1

5

paxdiablo 15 年前

我有几个问题要问你 :

它起作用了吗?
够快吗?
它是否可读/可维护?

如果这三个问题的答案都是肯定的,那么,除此之外,在我看来,进一步的改变确实是白费力气。不要仅仅为了重构而重构。

人们经常在预期可能发生的情况下改变事情(例如,你的“改变int”)。我更喜欢订阅雅格尼思想学校。担心这件事的正确时机是你做这件事的时候。

德米特定律是一种设计指南不是规则。在现实世界中,实用主义通常胜过教条主义。

2

0

Roboprog 15 年前

每个XXXEntryCreator和XXXEntry之间的关系是什么?我觉得我遗漏了一些东西,因为“创建者”只返回整数。

除此之外,您还使用了400行crud,找到了适合屏幕的东西,并且在各个步骤之间有一个相当可见的数据流。夸奖。(我在过去经历过强烈的阻力,因为我试图做出这样的改变——为什么人们会写N-100/1000行运行在其他的if drivel上?)

3

0

Carl Manaster pakore 15 年前

FindOrCreate 和 CreateOrUpdate 建议我多次通过可能会更简单(不知道代码的其余部分,我不知道它是否会降低性能,这是建议多次通过时常见的问题)。

如果您有一个循环来创建任何丢失的批次、供应商和买家(或三个循环),那么这个循环可以简化为

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

现在我看到创造者正在更新-对吗?将创建和更新责任划分为两个类是否有意义?

我开始觉得简单了。有帮助吗?