【问题标题】:Is it a code smell for one method to depend on another?一种方法依赖于另一种方法是一种代码味道吗?
【发布时间】:2009-07-29 08:46:06
【问题描述】:

我正在重构一个类,以便代码是可测试的(使用 NUnit 和 RhinoMocks 作为测试和隔离框架)并发现我发现自己的方法依赖于另一个方法(即它依赖于由其他方法)。类似于以下内容:

public class Impersonator
{
    private ImpersonationContext _context;

    public void Impersonate()
    {
        ...
        _context = GetContext();
        ...
    }

    public void UndoImpersonation()
    {
        if (_context != null)
            _someDepend.Undo();
    }
}

这意味着要测试UndoImpersonation,我需要通过调用Impersonate 来设置它(Impersonate 已经有几个单元测试来验证它的行为)。这对我来说很难闻,但从某种意义上说,从调用此类的代码的角度来看,这是有道理的:

public void ExerciseClassToTest(Impersonator c)
{
     try
     {
         if (NeedImpersonation())
         {
             c.Impersonate();
         }
         ...
     }
     finally
     {
         c.UndoImpersonation();
     }
}

如果我不尝试为UndoImpersonation 编写单元测试并发现自己必须通过调用其他公共方法来设置测试,我就不会解决这个问题。那么,这是一种难闻的气味吗?如果是,我该如何解决?

【问题讨论】:

  • 顺便说一句,这显示了单元测试的一个重要方面(?) - 好处:它让你思考你的设计...... :-)

标签: c# unit-testing tdd


【解决方案1】:

代码异味一定是我在编程世界中遇到过的最模糊的术语之一。对于一群以工程原理为荣的人来说,它在无法衡量的垃圾方面排名靠前,而对于程序员效率而言,它与每天的 LOC 一样无用。

无论如何,这是我的咆哮,感谢您的收听:-)

为了回答您的具体问题,我认为这问题。如果你测试有前置条件的东西,你需要确保首先为给定的测试用例设置了前置条件。

其中一个测试应该是当你调用它时会发生什么首先设置前置条件 - 它应该优雅地失败或者如果调用者没有设置它自己的前置条件懒得这么做了。

【讨论】:

  • 嗯,我确实发现代码闻起来非常有用。当然,它们只是经验法则,但很好(至少)。但是,是的,如果您接受要求先决条件是可以的(设计问题),那么测试就可以了。 +1 建议无需设置即可测试行为。
  • 我认为“代码异味”的问题在于程序员通常的缺陷,即把所有东西都乱扔垃圾。
  • +1 用于在不满足其前提条件的情况下添加调用方法的测试
【解决方案2】:

嗯,上下文太少了,看起来应该在构造函数中初始化 _someDepend。

在实例方法中初始化字段对我来说是个大问题。一个类一旦被构建就应该是完全可用的(即所有方法都有效);所以构造函数应该初始化所有实例变量。参见例如Ward Cunningham 的 wiki 中的 single step construction 页面。

在实例方法中初始化字段不好的原因主要是它对调用方法的方式施加了隐式顺序。在您的情况下, TheMethodIWantToTest 将根据是否首先调用 DoStuff 来做不同的事情。这通常不是您班级的用户所期望的,所以这很糟糕:-(。

也就是说,有时这种耦合可能是不可避免的(例如,如果一种方法获取文件句柄等资源,而需要另一种方法来释放它)。但如果可能的话,即使这样也应该用一种方法来处理。

如果没有更多上下文,很难说出适用于您的案例的内容。

【讨论】:

  • _someDepend 只是将类置于特定状态。所以不能在类构造函数中初始化
  • 好吧,出于同样的原因,拥有一个具有不同状态的类对我来说似乎是有问题的(同样的方法会根据这个“状态”做不同的事情)。很难说没有上下文,但你的班级可能做得太多了吗?也许每个“状态”一个类更合适(某些抽象类的可能子类)?
  • 我已经编辑了代码,希望能更好地了解上下文。
【解决方案3】:

如果您不认为可变对象本身就是代码异味,那么必须将对象置于测试所需的状态只是该测试设置的一部分。

【讨论】:

    【解决方案4】:

    这通常是不可避免的,例如在使用远程连接时 - 您必须先调用 Open(),然后才能调用 Close(),并且您不希望 Open() 在构造函数中自动发生。

    但是,您在执行此操作时要非常小心,以便该模式易于理解 - 例如,我认为大多数用户接受这种行为来处理任何事务,但当他们遇到 DoStuff()TheMethodIWantToTest() 时可能会感到惊讶(无论他们真正叫什么)。

    拥有一个代表当前状态的属性通常是最佳实践 - 再次查看远程或数据库连接,以了解一致理解的设计示例。

    最大的禁忌是物业永远不会发生这种情况。属性应该从不关心它们被调用的顺序。如果你有一个依赖于方法顺序的简单值,那么它应该是无参数方法而不是属性获取。

    【讨论】:

      【解决方案5】:

      是的,我认为在这种情况下存在代码异味。不是因为方法之间的依赖关系,而是因为对象的模糊标识。与其拥有一个可以处于不同角色状态的Impersonator,为什么不拥有一个不可变的Persona

      如果您需要不同的Persona,只需创建一个新的,而不是更改现有对象的状态。如果您之后需要进行一些清理,请将Persona 设为一次性。您可以将Impersonator 类保留为工厂:

      using (var persona = impersonator.createPersona(...))
      {
         // do something with the persona
      }
      

      【讨论】:

      • +1 好点。不要害怕创建新的对象实例,即使它们是一次性的。电子是可回收的:-)。
      【解决方案6】:

      回答这个问题:在面向对象编程中,方法相互调用(链接)是不可避免的,所以在我看来,测试一个调用另一个方法的方法并没有错。毕竟,单元测试可以是一个类,它是您正在测试的“单元”。

      链接的级别取决于对象的设计 - 您可以forkcascade

      • 分叉: classToTest1.SomeDependency.DoSomething()
      • 级联classToTest1.DoSomething()(内部会调用 SomeDependency.DoSomething)

      但正如其他人所提到的,绝对将您的状态初始化保留在构造函数中,据我所知,这可能会解决您的问题。

      【讨论】:

        猜你喜欢
        • 1970-01-01
        • 1970-01-01
        • 1970-01-01
        • 2020-06-28
        • 1970-01-01
        • 2011-04-17
        • 2014-07-21
        • 2017-10-30
        • 2013-02-10
        相关资源
        最近更新 更多