重构服务层类

Question

我的公司正处于单元测试的热潮中，我在重构服务层代码时遇到了一些麻烦。下面是我写的一段代码示例：

public class InvoiceCalculator:IInvoiceCalculator
{
   public CalculateInvoice(Invoice invoice)
   {
      foreach (InvoiceLine il in invoice.Lines)
      {
          UpdateLine(il);
      }
      //do a ton of other stuff here
   }

   private UpdateLine(InvoiceLine line)
   {
      line.Amount = line.Qty * line.Rate;
      //do a bunch of other stuff, including calls to other private methods
   }
}

在这个简化的情况下（它从一个有1个公共方法和约30个私有方法的1000行类中被缩减），我的老板说我应该能够分别测试我的CalculateInvoice和UpdateLine（UpdateLine实际上调用3个其他私有方法，并执行数据库调用）。但是我该如何做到呢？他建议的重构对我来说似乎有点复杂：

//Tiny part of original code
public class InvoiceCalculator:IInvoiceCalculator
{
   public ILineUpdater _lineUpdater;

   public InvoiceCalculator (ILineUpdater lineUpdater)
   {
      _lineUpdater = lineUpdater;
   }

   public CalculateInvoice(Invoice invoice)
   {
      foreach (InvoiceLine il in invoice.Lines)
      {
          _lineUpdater.UpdateLine(il);
      }
      //do a ton of other stuff here
   }
}

public class LineUpdater:ILineUpdater
{
   public UpdateLine(InvoiceLine line)
   {
      line.Amount = line.Qty * line.Rate;
      //do a bunch of other stuff
   }
}

我能看到现在依赖关系已经被打破了，我可以测试两个部分，但这也会从我的原始类中创建20-30个额外的类。我们只在一个地方计算发票，所以这些部分实际上并不可重用。这是进行此更改的正确方式吗？还是您建议我采取其他措施？

谢谢！

杰丝

- Beep beep

Answer 1

这是Feature Envy的一个示例：

line.Amount = line.Qty * line.Rate;

它可能应该看起来更像：

  var amount = line.CalculateAmount();

许多小类并没有什么问题，这不是关于可重用性，而是关于适应性。当您有许多单一职责的类时，更容易看到系统的行为，并在需求变化时进行更改。大型类具有交织的职责，这使得更改非常困难。

- Mike Valenty

---

我们所有的实体对象基本上都是DTO，因此所有的业务逻辑都在服务类中。但是发票行计算器中还有很多其他逻辑，我只展示了其中一部分。 - Beep beep

Answer 2

在我看来，这完全取决于UpdateLine()方法的“重要性”。如果它只是一个实现细节（例如，它可以轻松地内联到CalculateInvoice()方法中，唯一会影响可读性的是），那么您可能不需要将其与主类分开进行单元测试。

另一方面，如果UpdateLine()方法对业务逻辑有一定价值，如果您可以想象出在需要独立于类的其余部分更改此方法时（因此需要单独测试它）的情况，则应该继续将其重构为单独的LineUpdater类。

这样做可能不会让您最终拥有20-30个类，因为大多数私有方法实际上只是实现细节，不值得单独测试。

- Chriso

Answer 3

好的，就单元测试而言，你的老板更加正确：
他现在能够测试CalculateInvoice()而无需测试UpdateLine()函数。他可以传递模拟对象而不是真正的LineUpdater对象，并且仅测试CalculateInvoice()，而不是一大堆代码。
这样做对吗？这要取决于具体情况。你的老板希望进行真正的单元测试。在你的第一个示例中进行测试不是单元测试，而是集成测试。

单元测试有什么优势呢？
1）单元测试允许您仅测试一个方法或属性，而不受其他方法/数据库等的影响。
2）第二个优点- 单元测试执行速度更快（例如，您说UpdateLine使用数据库），因为它们不测试所有嵌套的方法。嵌套的方法可能是数据库调用，因此如果您有数千个测试，您的测试可能运行缓慢（几分钟）。
3）第三个优点：如果您的方法进行数据库调用，则有时需要设置数据库（将其填充为测试所必需的数据），这可能并不容易-也许您需要编写几页代码才能为测试准备数据库。通过单元测试，您可以使用模拟对象将数据库调用与正在测试的方法分离开来。

但是！我并不是说单元测试更好。它们只是不同而已。正如我所说，单元测试允许您在隔离环境中快速测试一个单元。集成测试更容易，可以让您测试不同方法和层的联合工作结果。老实说，我更喜欢集成测试 :)

此外，我有几个建议：
1）我认为拥有Amount字段不是一个好主意。因为它的值可以基于其他2个公共字段计算出来，所以Amount字段似乎是多余的。如果您仍然想这样做，我会将其作为只读属性返回Qty * Rate。
2）通常，一个由1000行组成的类可能意味着它设计得很糟糕，需要重构。

现在，我希望您更好地理解情况，并能够做出决定。此外，如果您了解情况，您可以与您的老板交谈，一起做出决定。

- nightcoder

Answer 4

好的，不错。我不确定InvoiceLine对象是否也包含了一些逻辑，否则你可能需要一个IInvoiceLine。 我有时也会有同样的问题。一方面，你想做正确的事情并对代码进行单元测试，但当涉及到数据库调用和文件写入时，设置第一个测试所需的所有测试对象、接口、断言以及测试数据层中不包含任何错误的测试都会导致大量额外的工作。因此，一个更多是“过程”而不是“单元”的测试通常更容易构建。 如果你有一个将来会经常改变的项目，并且这段代码有很多依赖项（也许其他程序读取文件或数据库数据），那么为代码的所有部分编写一个坚实的单元测试可能是很好的选择，投资时间是值得的。 但如果这个项目像我的最新客户说的那样，“让它上线，也许明年我们会微调一下，明年会有新的东西”，那么我就不会对自己太苛刻，去让所有的单元测试都运行起来。

米歇尔

- Michel

Answer 5

你老板的例子在我看来是合理的。

当设计任何场景时，我尽量考虑以下关键因素：

1. 单一职责原则

   一个类只应该因为一个原因而改变。

2. 每个新类是否有存在的必要性？

   创建类是为了什么目的，还是它们封装了有意义的逻辑部分？

3. 你能够测试每个代码片段吗？

在你的情况下，仅仅从名称上看，似乎你偏离了单一职责 - 你有一个IInvoiceCalculator，然而这个类也负责更新InvoiceLines。你不仅使得测试更新行为变得非常困难，而且当计算业务规则改变和更新规则改变时，你现在需要同时更改你的InvoiceCalculator类。

然后就是关于更新逻辑的问题 - 这种逻辑是否证明需要一个独立的类？这真的取决于具体情况，很难说，除非看到代码，但是你的老板希望将该逻辑放在测试之下，这表明它不仅仅是一个简单的一行调用数据层。

你说这个重构会创建大量的额外类（我理解你是指在所有业务实体中，因为我只看到了几个新类和它们的接口），但你必须考虑从中获得了什么。看起来你可以获得代码的完全可测试性，能够单独引入新的计算和更新逻辑，以及更清晰地封装业务逻辑的不同部分。

以上的收益当然需要进行成本效益分析，但既然你的老板要求这样做，那么他似乎很高兴这将会产生回报，抵消了实现这种代码的额外工作。

关于隔离测试的最后一点，也是你的老板设计的重要好处之一 - 公共方法与实际工作的代码越接近，就越容易为未经测试的系统部分注入存根或模拟。例如，如果您正在测试调用数据层的更新方法，则不希望测试数据层，因此通常会注入模拟。如果需要先将该模拟数据层通过所有计算器逻辑传递，那么您的测试设置将变得更加复杂，因为该模拟现在需要满足许多其他潜在需求，而这些需求与实际测试无关。

虽然这种方法最初需要额外的工作，但我认为大部分工作是思考设计的时间；在此之后，并且熟悉更基于注入的代码风格后，以这种方式构建的软件的原始实现时间实际上是可比较的。

- David Hall

Answer 6

你的方法是依赖注入的一个很好的例子，这样做可以让你使用模拟的ILineUpdater来高效地进行测试。

- ybakos