12

我继承了一个怪物。

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

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

这个嵌套的 if 有 715 行长。是的,这是正确的。七百五十条线。我不是代码分析专家,所以我下载了几个免费软件分析工具,得出的 McCabe 循环复杂度等级为 49。他们告诉我这是一个相当高的数字。与亚特兰大地区的花粉计数一样高,其中 100 是高标准,新闻称“今天的花粉计数为 1,523”。这是我有幸看到的箭头反模式最好的例子之一。在其最高处,缩进深度为 15 个制表符。

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

我花了一些时间寻找想法,但没有什么能让我站稳脚跟。例如,用保护条件代替级别是一种方法。我只有其中一个。一窝下来,十四走。

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

感谢您的任何想法。

4

7 回答 7

20

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

没有一件事能让你摆脱困境。状态机可能是您的代码采用的最终形式,但这不会帮助您实现目标,您也不应该在解决已经存在的混乱之前决定这样的解决方案。

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

重构不应改变代码的行为。记住这一点。这就是为什么您知道输入和输出数据集来验证您不会破坏事情的原因。这是你的安全网。

现在重构!

我做了几件我觉得有用的事情:

反转if语句

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

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

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

提取方法

我经常使用这个。拿一些复杂的多线块,摸索它,然后用它自己的方法把它推到一边。这让我更容易看到代码重复的地方。

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

现在,所有大牌模式都发挥作用的重大变化是当您开始考虑如何以适当的 OO 方式对其进行重构时。给这只猫剥皮的方法不止一种,而且会涉及多种模式。不知道您正在解析的这些文件的格式的详细信息,我只能提出一些有用的建议,这些建议可能是也可能不是最佳解决方案。

Refactoring to Patterns是一本很棒的书,可以帮助解释在这些情况下有用的模式。

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

于 2008-09-20T00:01:30.787 回答
2

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

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

画出你的状态,是什么导致了转变。处理记录的实际代码应该被分解出来,并在状态执行时调用(如果该特定状态需要它)。

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

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

于 2008-09-19T21:05:05.420 回答
2

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

于 2008-09-19T21:16:55.627 回答
2

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

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

在你进行的过程中,寻找任何看起来与其他方法不同的新长方法;将这些纳入新课程。只需使用一个现在只有一个方法的简单类。哎呀,使方法静态很好。不是因为你认为它们是好课程,而是因为你非常渴望某种组织。

经常签到,这样您就可以检查您的工作,稍后了解历史,准备好做一些“真正的工作”而无需合并,并为您的队友省去艰难合并的麻烦。

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

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

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

于 2008-09-20T00:17:22.573 回答
1

有时我将状态模式与堆栈结合起来。

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

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

于 2008-09-19T21:36:44.643 回答
1

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

于 2008-09-19T21:07:30.220 回答
1

在 Coding Horror 上有一篇非常好的博客文章。我只遇到过这种反模式一次,而且我几乎只是按照他的步骤进行操作。

于 2008-09-19T21:08:41.670 回答