1

我有一个方法，由于一些不直观的业务需求，需要大量评论。我不得不对其进行重构以使其不那么简洁（defactor？），但是当我说我评论过以下方法的地方对于确保业务需求不会意外更改时，请相信我：

public bool CheckBasedOnApplicableConditions()
{
    bool conditionOne;
    bool conditionTwo;

    // Comments to explain why BusinessLogic is used
    if (BusinessLogic())
    {
        //Comments discussing SecondaryBusinessLogic
        conditionOne = true;
        conditionTwo = SecondaryBusinessLogic();
    }
    else
    {
        //Comments discussing this scenario
        conditionOne = DifferentBusinessLogic();
        conditionTwo = !conditionOne;
    }

    //Comments about the CheckForCondition calls
    if (conditionOne && CheckForConditionOne())
    {
        return true;
    }
    if (conditionTwo && CheckForConditionTwo())
    {
        return true;
    }
    return false;
}

我真的很想在未来证明这种方法不会被开发人员破坏（又名我在一个月内）。我觉得强调业务需求包括必须检查两个条件中的至少一个是很重要的。所以我在最终返回之前添加了以下代码：

    //ReSharper normally points out this always evaluates to false,
    // but because an exception is being thrown it does not complain.
    if (!conditionOne && !conditionTwo)
    {
        throw new InvalidOperationException
            ("Condition One or Two must be applicable!");
    }

预期的效果是，如果开发人员不小心违反了业务需求，则会抛出异常，而不是默默地返回 false 并创建一个需要时间跟踪的错误。这不是我以前真正见过的模式，但是 ReSharper 在添加 throw 时自动抑制了它自己的“代码在启发式上无法访问”的警告。

我的问题是：这种方法是否有意想不到的副作用？即，性能损失，或我看不到的可维护性问题，或其他意外？我知道我可以重新考虑该方法以使其更稳定；但我担心这会以理解为代价。

4

2 回答 2

1

Henk 的回答让我朝着正确的方向前进：我非常专注于重构这一种方法，以至于我没有想到这实际上需要两种不同的方法。

现在我有：

public IEnumerable<Condition> DetermineConditionsToTest()
{
    //determine what conditions to return
}

public bool CheckIfAnyConditionIsMet(IEnumerable<Condition> conditionsToCheck)
{
    // check if any condition is met.
}

现在我对每个都有单元测试。教训是：如果你认为你不能对某些东西进行单元测试；继续分解它，直到可以为止。

于 2013-09-18T20:47:32.397 回答
1

自我测试代码在某种程度上是可以的，我建议System.Diagnostics.Debug.Assert()在这里。

但是验证这种逻辑条件最好通过外部测试来处理，即单元测试或（还不是很成熟的）代码合同。

而且由于您只能测试方法的结果，因此可以将其重构为更小的部分。

于 2013-09-18T18:54:46.457 回答