需要 Arrow Anti-Pattern 的重构思路

Need refactoring ideas for Arrow Anti-Pattern

提问人:Steve Owens 提问时间:9/20/2008 最后编辑:Gordon GustafsonSteve Owens 更新时间:5/13/2017 访问量:2104

问:

我继承了一个怪物。

它伪装成 .NET 1.1 应用程序处理符合医疗保健索赔付款 (ANSI 835) 标准的文本文件,但它是一个怪物。正在处理的信息与医疗保健索赔、EOB 和报销有关。这些文件由在前几个位置具有标识符的记录和根据该类型记录的规范格式化的数据字段组成。某些记录 ID 是控制段 ID,用于分隔与特定类型事务相关的记录组。

为了处理一个文件,我的小怪物读取第一条记录,确定即将发生的事务类型,然后根据它当前正在处理的事务类型开始处理其他记录。为此,它使用嵌套的 if。由于存在许多记录类型,因此需要做出许多决策。每个决策都涉及一些处理和 2-3 个其他决策,这些决策需要根据以前的决策做出。这意味着嵌套的 if 有很多嵌套。这就是我的问题所在。

如果嵌套的这个行长为 715 行。是的,没错。七百一十五行。我不是代码分析专家,所以我下载了几个免费软件分析工具,并得出了 McCabe Cyclomatic Complexity 评级 49。他们告诉我这是一个相当高的数字。在亚特兰大地区的花粉数量上很高,100是高的标准,新闻说“今天的花粉数量是1,523”。这是我见过的最好的 Arrow Anti-Pattern 例子之一。在最高处,缩进深度为 15 个选项卡。

我的问题是,你会建议什么方法来重构或重构这样的东西?

我花了一些时间寻找想法,但没有什么能给我一个好的立足点。例如,用保护条件代替级别是一种方法。我只有一个。一个巢穴下来,十四个去。

也许有一种设计模式可能会有所帮助。指挥链会是解决这个问题的一种方式吗?请记住,它必须保留在 .NET 1.1 中。

感谢您的任何想法。

if-statement 嵌套 重构 反模式

评论


答:

2赞 Philip Rieck 9/20/2008 #1

状态机似乎是合乎逻辑的起点,如果可以摆动它,请使用 WF(听起来好像你不能)。

你仍然可以在没有WF的情况下实现一个,你只需要自己做。但是,从一开始就将其视为状态机可能会为您提供更好的实现,然后创建一个过程怪物来检查每个操作的内部状态。

绘制出您的状态,导致过渡的原因。处理记录的实际代码应分解,并在状态执行时调用(如果该特定状态需要它)。

因此,State1 的 execute 调用您的“读取记录”,然后根据该记录转换到另一个状态。

下一个状态可能会读取多个记录并调用记录处理指令,然后转换回状态 1。

1赞 Nick Johnson 9/20/2008 #2

从描述来看,状态机可能是处理它的最佳方法。有一个枚举变量来存储当前状态,并将处理实现为对记录的循环,使用 switch 或 if 语句根据当前状态和输入数据选择要执行的操作。如果工作变得太庞大,您还可以使用函数指针轻松地将工作分派给基于状态的单独函数。

1赞 moswald 9/20/2008 #3

在 Coding Horror 上有一篇关于它的非常好的博客文章。我只遇到过一次这种反模式,我几乎只是跟随他的脚步。

2赞 SteveDonie 9/20/2008 #4

在这些情况下,我做的一件事是使用“组合方法”模式。请参阅 Jeremy Miller 关于此主题的博客文章。其基本思想是使用 IDE 中的重构工具来提取有意义的小方法。完成此操作后,您可以进一步重构和提取有意义的类。

1赞 erickson 9/20/2008 #5

有时我会将状态模式与堆栈组合在一起。

它适用于分层结构;父元素知道要推送到堆栈上以处理子元素的状态,但子元素不必知道有关其父元素的任何信息。换句话说,孩子不知道下一个状态是什么,它只是发出“完成”的信号,然后从堆栈中弹出。这有助于通过保持依赖关系单向性来将状态彼此分离。

它非常适合使用 SAX 解析器处理 XML(内容处理程序只是在输入和退出元素时推送和弹出状态以更改其行为)。EDI也应该适合这种方法。

评论

0赞 Steve Owens 9/20/2008
有趣的想法。我很难想象这在代码中会是什么样子。
20赞 craigb 9/20/2008 #6

这周我刚刚有一些遗留代码在工作,它们与您描述的相似(尽管没有那么可怕)。

没有一件事可以让你摆脱困境。状态机可能是你的代码所采用的最终形式,但这并不能帮助你到达那里,你也不应该在解开你已经拥有的混乱之前决定这样的解决方案。

我要采取的第一步是为现有代码编写一个测试。这个测试不是为了证明代码是正确的,而是为了确保你在开始重构时没有破坏某些东西。获取大量数据进行处理,将其提供给怪物,然后获得输出。这是你的试金石。如果你能用代码覆盖率工具做到这一点,你会看到你测试的内容没有涵盖。如果可以的话,构建一些人工记录,这些记录也将执行此代码,然后重复。一旦你觉得你已经完成了这个任务,输出数据就会成为你的测试的预期结果。

重构不应更改代码的行为。记住这一点。这就是为什么你有已知的输入和已知的输出数据集来验证你不会破坏东西。这是您的安全网。

现在重构!

我做的几件事我觉得很有用:

反转 if 语句

我遇到的一个大问题是,当我找不到相应的语句时,我只是阅读代码,我注意到很多块看起来像这样else

if (someCondition)
{
  100+ lines of code
  {
    ...
  }
}
else
{
  simple statement here
}

通过反转,我可以看到简单的情况,然后移动到更复杂的块上,知道另一个已经做了什么。这不是一个巨大的变化,但帮助我理解了。if

提取方法

我经常用这个。拿一些复杂的多行块,摸索它,然后用它自己的方法把它推到一边。这使我能够更轻松地查看哪里存在代码重复。

现在,希望你没有破坏你的代码(测试仍然通过,对吧?),并且你有更可读和更好地理解的过程代码。看,它已经改进了!但是你之前写的那个测试还不够好......它只告诉你你复制了原始代码的功能(错误和所有),而这只是你所覆盖的那一行,因为我相信你会发现你无法弄清楚如何命中或永远无法命中的代码块(我在我的工作中都见过)。

现在,所有大牌模式发挥作用的重大变化是,当你开始研究如何以适当的面向对象方式重构它时。给这只猫剥皮的方法不止一种,它会涉及多种模式。不知道您正在解析的这些文件的格式的详细信息,我只能抛出一些有用的建议,这些建议可能是也可能不是最佳解决方案。

《重构模式》是一本很棒的书,可以帮助解释在这些情况下有用的模式。

你想吃一头大象,除了一口一口,别无他法。祝你好运。

评论

1赞 troelskn 9/29/2008
赞成大象的类比。这就是重构的本质;这是一点一点的。
0赞 Huperniketes 11/29/2011
一个很好的重构指南,但有几点。我建议将状态表定义为使用试验数据执行遗留代码以帮助 grok 和重新实现代码。我还建议不要反转 if 语句,因为可能会使生成的条件表达式出错。
2赞 Jay Bazuzi 9/20/2008 #7

我会从无拘无束地使用提取方法开始。如果当前 Visual Studio IDE 中没有它,则可以获取第三方加载项,或将项目加载到较新的 VS 中。(它会尝试升级您的项目,但您会小心地忽略这些更改,而不是签入它们。

你说你的代码缩进了 15 个级别。从大约 1/2 出路开始,然后提取方法。如果你能想出一个好名字,就用它,但如果你不能,无论如何都要提取。再次劈成两半。你在这里不会追求理想的结构;你试图将代码分解成适合你大脑的碎片。我的脑袋不是很大,所以我会不停地打破,直到它不再疼为止。

在你走的时候,寻找任何新的长方法,这些方法似乎与其他方法不同;将这些放入新类中。只需使用一个目前只有一个方法的简单类即可。哎呀,使方法静态很好。不是因为你认为他们是好班级,而是因为你非常渴望某个组织。

在你去的时候经常签到,这样你就可以检查你的工作,以后了解历史,准备好做一些不需要合并的“真正的工作”,并省去你的队友硬合并的麻烦。

最终,您需要返回并确保方法名称正确,您创建的方法集有意义,清理新类等。

如果您拥有高度可靠的提取方法工具,则无需良好的自动化测试即可逃脱。(例如,我会相信 VS。否则,请确保您没有破坏东西,否则您最终会比开始时更糟:使用一个根本不起作用的程序。

在这里,配对伙伴会有所帮助。