重构具有大量级别之间依赖关系的内部循环

Question

我有以下伪代码

using (some web service/disposable object)
{
    list1 = service.get1();

    list2 = service.get2();

    for (item2 in list2)
    {
        list3 = service.get3(depending on item2);

        for (item3 in list3)
        {
            list4 = service.get4(depending on item3 and list1);

            for (item4 in list4)
            {
                ...
            }
        }
    }
}

整个代码有 500 行，在 for 语句中有很多逻辑。问题是将其重构为可读和可维护的代码，并作为类似情况的最佳实践。以下是我目前找到的可能的解决方案。

1：拆分成方法 考虑到我们将每个 for 提取到它自己的方法中，为了提高可读性，我们最终得到 method2、method3 和 method4。每个方法都有自己的参数和依赖关系，这很好，除了 method4。 Method4 依赖于 list1，这意味着 list1 也必须传递给方法 2 和 3。在我看来，这变得不可读了。任何查看 method2 的开发人员都会意识到其中的 list1 没有意义，因此他必须向下查看链，直到 method4 才能真正意识到依赖 -> 低效。那么，如果 list4 或 item4 发生变化并且不再需要依赖 list1 会发生什么？我必须删除方法 4 的 list1 参数（当然应该这样做），但也必须删除方法 2 和 3 的参数（超出更改范围）-> 再次效率低下。另一个副作用是，在很多依赖和多层次的情况下，传递的参数数量会迅速增加。想想如果list4还依赖list11、list12和list13，都是在list1级别创建的。

2：保持长单方法 优点是每个列表和项目都可以访问每个父列表和项目，这使得进一步的更改成为一行。如果 list4 不再依赖于 list1，只需 remove/replace 代码，无需更改任何其他内容。显然，问题是该方法是几百行，我们都知道这不好。

3。两全其美？ 这个想法是只将每个 for 的内部逻辑部分拆分成方法。这样 main 方法将减少，我们获得可读性和可能的可维护性。将 for 留在 main 方法中，我们仍然能够将每个依赖项作为一行来访问。我们最终得到这样的结果：

for (item2 in list2)
{
    compute();

    list3 = service.get3(depending on item2);

    for (item3 in list3)
    {
        compute(item2, eventually item1)

        list4 = service.get4(depending on item3 and list1);

        for (item4 in list4)
        {
            ...
        }
    }
}

但是还有一个问题。为了便于阅读，您如何命名这些 compute 方法？假设我有一个局部变量 instanceA 或类型 ClassA，我从 item2 填充它。 Class A 包含一个名为 LastItem4 的属性，它需要按创建日期或其他顺序保留最后一个 item4。如果我使用计算来创建 instanceA，那么这个计算只是部分的。因为 LastItem4 属性需要在不同的计算方法中填充到 item4 级别。那么我如何调用这两种计算方法来建议我实际在做什么呢？我认为很难回答。

4. #地区 保留一个长方法但使用区域。这是一些编程语言中严格使用的功能，在我看来就像一个补丁，而不是最佳实践，但也许只有我一个人如此。

我想知道您将如何进行重构？请注意，它可能比这更复杂。这是一个服务的事实有点误导，这个想法是我真的不喜欢使用一次性对象作为方法参数，因为如果不阅读实际的方法代码你就不知道意图。但我在评论中期待这一点，因为其他人可能会有不同的看法。

为了举例，我们假设该服务是第 3 方服务，无法更改其工作方式。

Answer 1

不要使用区域。如果您打印，如果您使用 CodeMap 等等，它们将无济于事。

将 list1、list2、..设为私有 class 变量。

为每个 for() 循环创建一个方法：

private void method3()
{
    for (item3 in list3)
    {
        list4 = service.get4(depending on item3 and list1);
        method4();
    }
}

对每种方法提出一些好的意见...

这有额外的好处，可以对每个 methodX()

进行很好的单元测试

Answer 2

您似乎在寻找一个笼统的答案，而不是一个特定的答案。一般答案的关键是分离职责并努力实现单一职责原则。

让我从你的方法太多事情开始。这么多东西理想情况下 class 也不应该做。它应该在 classes.

组的帮助下完成

罗伯特C.Martin曾经说过"Classes tends to hide in methods"，这里就是这样。如果你的方法很大，你可以将它重构为更小的方法，但如果整个方法都需要一个局部变量，那么该方法可能应该放在它自己的 class.

中

如果您需要同时访问某些字段，则它们必须在一起（我的意思是在同一类型中）。所以你所有的 List1、List2、List3、List4 都应该放在 class 中，而不是作为方法参数。

鉴于无法修改Serviceclass（假设是第三方API）。没关系，您始终可以创建自己的 class 来包装上述服务。

所以你基本上 class 如下所示：

public class ServiceWrapper
{
    private IList<string> list1;
    private IList<string> list2;
    private IList<string> list3;
    private IList<string> list4;

    private readonly Service1 service;
    public ServiceWrapper(Service1 service)
    {
        this.service = service;
    }

    public SomeClass GetSomething()
    {
        list1 = service.Get1();
        list2 = service.Get2();

        foreach (var item2 in list2)
        {
            PopulateMore(item2);
        }

        return result;//Return some computed result
    }

    private void PopulateMore(string item2)
    {
        list3 = service.Get3(item2);

        foreach (var item3 in list3)
        {
            PopulateEvenMore(item3);
        }
    }

    private void PopulateEvenMore(string item3)
    {
        list4 = service.Get4(item3);

        foreach (var item4 in list4)
        {
            //And so on
        }
    }
}

鉴于您的服务看起来像这样

public class Service1 : IDisposable
{
    public void Dispose()
    {
        throw new NotImplementedException();
    }

    public IList<string> Get1()
    {
        throw new NotImplementedException();
    }

    public IList<string> Get2()
    {
        throw new NotImplementedException();
    }

    public IList<string> Get3(string item2)
    {
        throw new NotImplementedException();
    }

    public IList<string> Get4(string item3)
    {
        throw new NotImplementedException();
    }
}

所以你很长的方法现在变得很小了。

public void YourVeryLongMethodBecomesVerySmall()
{
    using (Service1 service = new Service1())
    {
        SomeClass result = new ServiceWrapper(service).GetSomething();
    }
}

正如您在评论中所说，如果您在这些方法中还有一些其他职责，您需要将这些职责分开并单独移动 class。例如，如果你想解析一些东西，那应该放在一些 Parser class 里面， ServiceWrapper 应该使用解析器来完成工作（最好有一些抽象和松耦合）。

您应该尽可能多地提取方法和 classes，这样提取就没有意义了。

Answer 3

简短的回答，这取决于您的问题领域以及您可以将您的方法拆分成多少有意义的动词，但根据我的经验，通常选项 3 是最佳选择。

关于#region，不要使用它。它最适合用于将生成的代码与您的代码分开。使用区域 "structure" 你的代码就像把垃圾放在地毯下，它仍然在那里，你能感觉到它。

还有一点要注意，您可能正面临与集合相关的问题，像 Guava (google-collections) 这样的好库会对您有很大帮助。此外，使用 Java 8 个 lambda 功能或 .Net LINQ 可以帮助您重写代码并使其更具表现力。

番石榴：https://code.google.com/p/guava-libraries/
Java 8 流与 .Net LINQ：http://blog.informatech.cr/2013/03/24/java-streams-preview-vs-net-linq/

Answer 4

我最终结合使用了问题中描述的解决方案 3 和 DTO。这不一定是最佳选择，但在可读性、灵活性和缩放性方面优于其他选择。

重构具有大量级别之间依赖关系的内部循环

Refactoring inner loops with lots of dependencies between levels

c#

java

convention

for-loop

nested