查找(并提取)复杂的关联以查找规则违反

这不是一个简单的任务，所以我要做的第一件事是定义一个接口，看看我在这里需要什么:

interface IViolationFinder
{
    IEnumerable<Violation> Search(
        IEnumerable<RuleDefinition> ruleDefinitions, 
        IEnumerable<Request> requests);
}

现在我们清楚地看到我们需要实现什么。因为你的搜索逻辑非常复杂，我认为你不应该用一个linq来表达它。你可以，但你不应该。两个嵌套的foreach循环，其中嵌入了linq，这是非常讨厌的，我不认为linq本身会更干净。

你需要的是在你的实现中创建更多的方法。这会增加可读性。所以简单的实现是这样的(这是你的):

class ViolationFinder : IViolationFinder
{
    public IEnumerable<Violation> Search(IEnumerable<RuleDefinition> ruleDefinitions, IEnumerable<Request> requests)
    {
        var violations = new List<Violation>();
        foreach (var rule in ruleDefinitions)
        {
            var requestsMatchingType = requests.Where(r => r.TypeOfThisRequest == rule.ConcerningRequestType);
            foreach (var firstRequest in requestsMatchingType)
            {
                var collidingRequest = requests.FirstOrDefault(secondRequest =>
                    secondRequest.TimeIndex > firstRequest.TimeIndex &&
                    Math.Abs(secondRequest.TimeIndex - firstRequest.TimeIndex) < rule.MinimumDistanceBetweenRequests);
                if (collidingRequest != null)
                {
                    violations.Add(new Violation
                    {
                        ViolatedRule = rule,
                        FirstRequest = firstRequest,
                        SecondRequest = collidingRequest
                    });
                }
            }
        }
        return violations;
    }
}

你可以开始重构它。让我们提取最明显的部分，而不是考虑一种方法:

class ViolationFinder : IViolationFinder
{
    public IEnumerable<Violation> Search(IEnumerable<RuleDefinition> ruleDefinitions, IEnumerable<Request> requests)
    {
        var violations = new List<Violation>();
        foreach (RuleDefinition rule in ruleDefinitions)
        {
            IEnumerable<Request> requestsMatchingType = requests.Where(r => r.TypeOfThisRequest == rule.ConcerningRequestType);
            violations.AddRange(
                FindViolationsInRequests(requestsMatchingType, requests, rule));
        }
        return violations;
    }
    private IEnumerable<Violation> FindViolationsInRequests(
        IEnumerable<Request> matchingRequests,
        IEnumerable<Request> allRequest,
        RuleDefinition rule)
    {
        foreach (Request firstRequest in matchingRequests)
        {
            var collidingRequest = allRequest.FirstOrDefault(secondRequest =>
                secondRequest.TimeIndex > firstRequest.TimeIndex &&
                Math.Abs(secondRequest.TimeIndex - firstRequest.TimeIndex) < rule.MinimumDistanceBetweenRequests);
            if (collidingRequest != null)
            {
                yield return new Violation
                {
                    ViolatedRule = rule,
                    FirstRequest = firstRequest,
                    SecondRequest = collidingRequest
                };
            }
        }
    }
}

Search几乎是干净的，但是我们看到FindViolationsInRequests获取每个请求，因此传递过滤请求列表的规则是毫无用处的。所以我们这样做:

class ViolationFinder : IViolationFinder
{
    public IEnumerable<Violation> Search(IEnumerable<RuleDefinition> ruleDefinitions, IEnumerable<Request> requests)
    {
        var violations = new List<Violation>();
        foreach (RuleDefinition rule in ruleDefinitions)
        {
            violations.AddRange(FindViolationsInRequests(requests, rule));
        }
        return violations;
    }
    private IEnumerable<Violation> FindViolationsInRequests(
        IEnumerable<Request> allRequest,
        RuleDefinition rule)
    {
        foreach (Request firstRequest in FindMatchingRequests(allRequest, rule))
        {
            var collidingRequest = allRequest.FirstOrDefault(secondRequest =>
                secondRequest.TimeIndex > firstRequest.TimeIndex &&
                Math.Abs(secondRequest.TimeIndex - firstRequest.TimeIndex) < rule.MinimumDistanceBetweenRequests);
            if (collidingRequest != null)
            {
                yield return new Violation
                {
                    ViolatedRule = rule,
                    FirstRequest = firstRequest,
                    SecondRequest = collidingRequest
                };
            }
        }
    }
    private IEnumerable<Request> FindMatchingRequests(IEnumerable<Request> requests, RuleDefinition rule)
    {
        return requests.Where(r => r.TypeOfThisRequest == rule.ConcerningRequestType);
    }
}

下一件事是

    var collidingRequest = allRequest.FirstOrDefault(secondRequest =>
        secondRequest.TimeIndex > firstRequest.TimeIndex &&
        Math.Abs(secondRequest.TimeIndex - firstRequest.TimeIndex) < rule.MinimumDistanceBetweenRequests);

足够复杂，可以为它创建一些方法:

class ViolationFinder : IViolationFinder
{
    public IEnumerable<Violation> Search(IEnumerable<RuleDefinition> ruleDefinitions, IEnumerable<Request> requests)
    {
        var violations = new List<Violation>();
        foreach (RuleDefinition rule in ruleDefinitions)
        {
            violations.AddRange(FindViolationsInRequests(requests, rule));
        }
        return violations;
    }
    private IEnumerable<Violation> FindViolationsInRequests(
        IEnumerable<Request> allRequest,
        RuleDefinition rule)
    {
        foreach (Request firstRequest in FindMatchingRequests(allRequest, rule))
        {
            Request collidingRequest = FindCollidingRequest(allRequest, firstRequest, rule.MinimumDistanceBetweenRequests);
            if (collidingRequest != null)
            {
                yield return new Violation
                {
                    ViolatedRule = rule,
                    FirstRequest = firstRequest,
                    SecondRequest = collidingRequest
                };
            }
        }
    }
    private IEnumerable<Request> FindMatchingRequests(IEnumerable<Request> requests, RuleDefinition rule)
    {
        return requests.Where(r => r.TypeOfThisRequest == rule.ConcerningRequestType);
    }
    private Request FindCollidingRequest(IEnumerable<Request> requests, Request firstRequest, int minimumDistanceBetweenRequests)
    {
        return requests.FirstOrDefault(secondRequest => IsCollidingRequest(firstRequest, secondRequest, minimumDistanceBetweenRequests));
    }
    private bool IsCollidingRequest(Request firstRequest, Request secondRequest, int minimumDistanceBetweenRequests)
    {
        return secondRequest.TimeIndex > firstRequest.TimeIndex &&
               Math.Abs(secondRequest.TimeIndex - firstRequest.TimeIndex) < minimumDistanceBetweenRequests;
    }
}

好了，越来越干净了。我几乎可以很容易地说出每种方法的目的。再多做一点工作，你就会得到这样的结果:

class ViolationFinder : IViolationFinder
{
    public IEnumerable<Violation> Search(IEnumerable<RuleDefinition> ruleDefinitions, IEnumerable<Request> requests)
    {
        List<Request> requestList = requests.ToList();
        return ruleDefinitions.SelectMany(rule => FindViolationsInRequests(requestList, rule));
    }
    private IEnumerable<Violation> FindViolationsInRequests(IEnumerable<Request> allRequest, RuleDefinition rule)
    {
        return FindMatchingRequests(allRequest, rule)
                .Select(firstRequest => FindSingleViolation(allRequest, firstRequest, rule))
                .Where(violation => violation != null);
    }
    private Violation FindSingleViolation(IEnumerable<Request> allRequest, Request request, RuleDefinition rule)
    {
        Request collidingRequest = FindCollidingRequest(allRequest, request, rule.MinimumDistanceBetweenRequests);
        if (collidingRequest != null)
        {
            return new Violation
            {
                ViolatedRule = rule,
                FirstRequest = request,
                SecondRequest = collidingRequest
            };
        }
        return null;
    }
    private IEnumerable<Request> FindMatchingRequests(IEnumerable<Request> requests, RuleDefinition rule)
    {
        return requests.Where(r => r.TypeOfThisRequest == rule.ConcerningRequestType);
    }
    private Request FindCollidingRequest(IEnumerable<Request> requests, Request firstRequest, int minimumDistanceBetweenRequests)
    {
        return requests.FirstOrDefault(secondRequest => IsCollidingRequest(firstRequest, secondRequest, minimumDistanceBetweenRequests));
    }
    private bool IsCollidingRequest(Request firstRequest, Request secondRequest, int minimumDistanceBetweenRequests)
    {
        return secondRequest.TimeIndex > firstRequest.TimeIndex &&
               Math.Abs(secondRequest.TimeIndex - firstRequest.TimeIndex) < minimumDistanceBetweenRequests;
    }
}

请注意，单一职责原则也适用于方法。除了Search方法之外，其他都是私有实现的一部分，但是正如您可能看到的，每个处理部分都有一个带名称的方法。每个方法都有它的单一职责，所以你可以更容易地阅读实现。

• 搜索(公众)
• FindViolationsInRequests
• FindSingleViolation
• FindMatchingRequests
• FindCollidingRequest
• IsCollidingRequest

这些是这个实现的单元。

如果您为原始实现编写单元测试，然后才开始重构，那么重构过程将更加安全。那你就知道你不会打破你的逻辑。如果您针对第一个变量(当我将您的完整代码放入Search方法中时)编写单元测试，那么针对接口编写单元测试就可以了。

另一个小而重要的部分是:

public IEnumerable<Violation> Search(IEnumerable<RuleDefinition> ruleDefinitions, IEnumerable<Request> requests)
{
    List<Request> requestList = requests.ToList();
    return ruleDefinitions.SelectMany(rule => FindViolationsInRequests(requestList, rule));
}

我从项目中制作一个列表，所以我绝对肯定我不会枚举IEnumerable不止一次(这可能会导致某些实现问题，想想IQueryable)。

如果您不反对使用查询表达式，那么您可以将您的实现写成:

var violations = from rule in ruleDefinitions
                 join r1 in requests on rule.ConcerningRequestType equals r1.TypeOfThisRequest
                 join r2 in requests on rule.ConcerningRequestType equals r2.TypeOfThisRequest
                 where r1 != r2 &&
                       r2.TimeIndex > r1.TimeIndex &&
                       Math.Abs(r2.TimeIndex - r1.TimeIndex) < rule.MinimumDistanceBetweenRequests
                 select new Violation() { FirstRequest = r1, SecondRequest = r2, ViolatedRule = rule };