帮我重构这个循环
我正在重新设计现有的课程。 在本课程中，大约有 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）。 以下以 p 作为参数的进程需要此 id。
这比我开始的地方有所改进，但我对包含此循环的类有一种不安的感觉。
- 它似乎不是一个域对象，它更像是一个“Manager”或“Controller”类型的类。
- 它似乎介于批处理程序和供应商BatchEntryCreator（以及其他类）之间。 目前仅传递一个 int ，但如果它发生变化，则所有三个类都需要更改。 这似乎违反了摄魂怪定律。
有什么建议吗，或者这样可以吗？ 实际语言是java。
I am working on the redesign of an existing class. In this class about a 400-line while loop that does most of the work. The body of the loop is a minefield of if statements, variable assignments and there is a "continue" in the middle somewhere. The purpose of the loop is hard to understand.
In pseudocode, here's where I'm at the redesign:
/* 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();
Inside FindOrCreateBatch() it figures out using some rules if a new batch needs to be created or if an existing one can be used. The different implementations of this interface will have different rules for how it finds them, etc. The return value is the surrogate key (id) from the database of the payment batch that it found or created. This id is required by following processes that take p as a parameter.
This is an improvement over where I started, but I have an uneasy feeling about the class containing this loop.
- It doesn't seem to a be a domain object, it's more of a "Manager" or "Controller" type class.
- It seems to be getting inbetween batcher and supplierBatchEntryCreator (and the other classes). At the moment only an int is passed, but if that changes all three classes need to change. This seems like a Law of Dementer violation.
Any suggestions, or is this ok? The actual language is java.
如果你对这篇内容有疑问，欢迎到本站社区发帖提问 参与讨论，获取更多帮助，或者扫码二维码加入 Web 技术交流群。
绑定邮箱获取回复消息
由于您还没有绑定你的真实邮箱，如果其他用户或者作者回复了您的评论，将不能在第一时间通知您！
发布评论
评论（3）
我有几个问题要问你：
如果这三个问题的答案都是肯定的，那么除此之外，在我看来，进一步的改变实际上只是浪费精力。 不要仅仅为了重构而重构。
人们常常会根据可能发生的情况来改变事情（例如你的“改变整数”）。 我更倾向于认同 YAGNI 学派。 当你这样做的时候，就是担心这个问题的正确时机。
德墨忒尔法则是一个设计指南，而不是规则。 在现实世界中，实用主义通常胜过教条主义:-)
I have a couple of questions to ask you:
If the answer to all three is yes then, beyond that, further changes are really just wasted effort in my opinion. Don't refactor just for the sake of refactoring.
Far too often people change things in anticipation of what might be (your "changing int" for example). I prefer to subscribe to the YAGNI school of thought. The right time to worry about that is when you do it.
And the Law of Demeter is a design guideline, not a rule. In the real world, pragmatism usually beats dogmatism :-)
每个XXXEntryCreator和XXXEntry之间是什么关系？ 我觉得我错过了一些东西，因为“创建者”只返回整数。
除此之外，您将 400 行粗略的内容简化为适合屏幕的内容，并且在步骤之间具有相当可见的数据流。 荣誉。 （过去我在尝试做出这样的改变时遇到了强烈的阻力——为什么人们要写 N-100/1000 行连续的 else-if 胡言乱语？）
What is the relationship between each XXXEntryCreator and XXXEntry? I feel like I am missing something, since the "Creators" only return integers.
Beyond that, you took 400 lines of crud down to something that fits on a screen, and has a reasonably visible data flow between steps. Kudos. (I have experienced strong resistance in the past for trying to make such changes -- why do people write N-100/1000 line run-on else-if drivel?)
FindOrCreate和CreateOrUpdate向我建议，也许多次传递会更简单（并且不知道代码的其余部分，我不知道它是否会降低性能，这是当建议多次通过时，这是一个常见的问题）。如果您有一个循环来创建任何缺失的批次、供应商和买家（或三个循环），那么该循环可以简化为“
现在我看到创建者正在更新 - 是这样吗？” 将创建和更新职责分为两个类是否有意义？
对我来说，它开始看起来更简单了。 有帮助吗？
FindOrCreateandCreateOrUpdatesuggest to me that maybe multiple passes would be simpler (and not knowing the rest of the code, I can't know if it would degrade performance, which is a common concern raised when multiple passes are suggested).If you had one loop to create any missing batches, suppliers, and buyers (or three loops), then this loop could be reduced to
Now I see that the Creator's are updating - is that right? Does splitting the creation and update responsibility into two classes make sense, perhaps?
It's starting to look a little simpler to me. Does it help?