1

我正在处理一些写得不太好的代码，并且涉及一些我希望重构的相当复杂的逻辑。主题是验证规则和报告潜在的违规行为。不幸的是，课程设计相当奇怪，所以我遇到了一些 IEnumerable 挑战。

作为一个简化的示例，我有以下内容：

IEnumerable<RuleDefinition>
IEnumerable<Request>

在哪里

public class RuleDefinition
{
    public RequestType ConcerningRequestType { get; set; }
    public int MinimumDistanceBetweenRequests { get; set; }
}

public class Request
{
    public int TimeIndex { get; set; }
    public RequestType TypeOfThisRequest { get; set; }
}

显然，当请求类型匹配并且两个请求之间的时间间隔（TimeIndex）太短时，就违反了规则。现在，我想提取：

  • 如果有违规行为（这很容易）
  • 违反了哪些规则
  • 哪些请求违反了规则

所以在我们的例子中，我想获得这样的东西：

public class Violation
{
    public RuleDefinition ViolatedRule { get; set; }
    public Request FirstRequest { get; set; }
    public Request SecondRequest { get; set; }
}

我认为这是一个相当简单的问题，但是我没有想出一个可以称为可读性和可维护性的解决方案。我尝试了各种事情..结果总是一团糟（我只是试图实现这个例子，这很糟糕）

在这种情况下使用的任何想法，模式？（Resharper 经常正确地建议 .SelectMany，但这会让事情变得更不可读）

编辑：这是我漫长而丑陋的实现。;)

var ruleDefinitions = new List<RuleDefinition>
{ 
    new RuleDefinition { 
        ConcerningRequestType = RequestType.Exclusive, 
        MinimumDistanceBetweenRequests = 2} 
};
var requests = new List<Request>()
    {
        new Request { TimeIndex = 1, TypeOfThisRequest = RequestType.Normal },
        new Request { TimeIndex = 1, TypeOfThisRequest = RequestType.Normal },
        new Request { TimeIndex = 2, TypeOfThisRequest = RequestType.Normal },

        new Request { TimeIndex = 3, TypeOfThisRequest = RequestType.Exclusive },
        new Request { TimeIndex = 4, TypeOfThisRequest = RequestType.Exclusive },
    };

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
                });
        }
    }
}
Console.WriteLine("found {0} violations.", violations.Count());
Console.ReadKey();
4

2 回答 2

2

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

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

现在我们清楚地看到我们需要实现什么。因为您的搜索逻辑非常复杂，我认为您不应该使用单个 linq 来表达它。你可以，但你不应该。嵌入了 linq 的两个嵌套 foreach 循环非常讨厌，我认为使用 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
                };
            }
        }
    }
}

搜索几乎是干净的，但我们看到 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
  • 查找匹配请求
  • 查找碰撞请求
  • 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）。

于 2013-03-22T18:03:26.817 回答
0

如果您不反对使用查询表达式，那么您可以将实现编写为：

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 };
于 2013-03-22T18:50:21.677 回答