我的代码是否过于程序化？答案

【问题标题】：Is my code too procedural?我的代码是否过于程序化？
【发布时间】：2011-05-13 22:42:31
【问题描述】：

最近有人看了我的代码并评论说它太程序化了。需要明确的是，他们看到的代码并不多 - 只是一个清楚地概述了应用程序中所采取的逻辑步骤的部分。

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap())) {
    loadDataSources();
    initEngine();
    loadLiveData();
    processX();
    copyIds();
    addX();
    processY();
    copyIds();
    addY();
    pauseY();
    resumeY();
    setParameters();
}

然后这些不同的方法会创建一大堆不同的对象，并根据需要在这些对象上调用各种方法。

我的问题是 - 是否有一段代码清楚地驱动您的应用程序，例如这样，表明过程编程，如果是这样，那么实现相同结果的更面向对象的方法是什么？

非常感谢所有 cmets！

【问题讨论】：

我们有时似乎忘记了 OOP 的借口是更好的可维护性。所以问问你自己，你的代码对其他从未见过的人来说是否清楚，他们是否很难做出改变。可以在您的环境中的其他地方使用此代码并稍加修改吗？（如果您要将此模块剪切并粘贴到其他地方，也许 OO 可以在这里提供帮助）...在不了解您的应用程序以及所有功能的含义的情况下，如果有必要担心的话，很难说清楚。
这一切都归结为可重用性。我想这都是在一些 main() 方法中，所有这些方法都是静态的。现在，如果您必须更改某些内容，也许您有一个额外的 Z 字段，现在您需要添加进程 Z 和恢复 Z 等等。对于您的代码，如果不进行重大重构，将很难做到这一点。如果你走的是面向对象的路线，那么它就像启动另一个类来处理它一样简单。这就是 OOP 的力量。
我感觉有很多全局变量。此代码 100% 无法测试。

标签： java oop procedural-programming

【解决方案1】：

嗯，这段代码所在的类显然有太多的责任。我不会将所有这些东西隐藏在外观中，而是将所有与某些 ftp 引擎、数据源和其他实体相关的东西都放在单个上帝对象（反模式）中，应该有一个业务流程所有这些类型的实体。

所以代码看起来更像这样：

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap())) {
    datasource.load();
    engine.init();
    data.load();
    engine.processX(data);
    data.copyIds()
    foo.addX();
    engine.processY();
    // ...
}

数据源、引擎和所有其他组件都可能被注入到您的业务流程中，因此 a) 测试变得更容易，b) 交换实现被简化，c) 代码重用成为可能。

请注意，看起来程序化的代码并不总是很糟糕：

class Person {
    public void getMilk() 
    {
        go(kitchen);
        Glass glass = cupboard.getGlass(); 
        fridge.open(); 
        Milk milk = fridge.getMilk(); 
        use(glass, milk);
        drink(glass);
    } 
    // More person-ish stuff
}

虽然该代码显然看起来是程序化的，但它可能没问题。它完全清楚这里发生了什么，并且不需要任何文档（来自马丁的干净代码鼓励这样的代码）。只需牢记单一职责原则和所有其他基本 OOP 规则即可。

【讨论】：

最好编辑“goto”方法调用，因为它是 java 中的保留字，会造成混淆。仅仅因为您的“goto”语句被蓝色的语法格式化程序捕获。
那只是一段毫无意义的伪代码，但是是的，你说得对。
我知道你的意思，但它只是刺激。感谢您的编辑。在这里你的 +1
谢谢你的提示，我没明白；）
@mohamed 这是一个公平的观点，但我认为杯子不应该负责被牛奶填满。也许根本不应该有 getMilk 方法，只是一种通用的饮料（Drink d）？设计很有趣！

【解决方案2】：

哇！它看起来不像 OO 风格。像这样怎么样：

    ConnectionData cData = new ConnectionData(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap());


    if(downloadFeeds(cData)) {
      MyJobFacade fc = new MyJobFacade(); 
      fc.doYourJob();
    }

MyJobFacade.java

public class MyJobFacade {

    public void doYourJob() {
          /* all your do operations maybe on different objects */
    }
}

顺便说一下外观模式 http://en.wikipedia.org/wiki/Facade_pattern

【讨论】：

外观模式可能会在您想隐藏一些丑陋的 api 或调用广泛分布在您的问题域中的代码时有所帮助，但如果示例代码是一个真实的业务流程，我不会尝试把它藏起来。在这种情况下，分解所有职责可能足以创建干净、漂亮的代码。

【解决方案3】：

我的问题是 - 是一段代码清楚地驱动你的应用程序，例如这个，表明程序编程，

不能说这个代码片段是否“过于程序化”。

这些调用可能都是针对当前对象的实例方法，对单独的对象或当前实例的实例变量进行操作。这些将使代码至少在某种程度上是OO的。
如果这些方法是static，那么代码确实是程序性的。这是否是一件坏事取决于方法是否正在访问和更新存储在static 字段中的状态。（从名字来看，他们可能需要这样做。）

如果是这样，实现相同结果的更面向对象的方法是什么？

不看其余代码很难说，但图中似乎有一些隐含的对象；例如

数据源和（可能）数据源管理器或注册表
某种engine
保存实时数据、X 和 Y 的东西
等等。

其中一些可能需要是类（如果它们还不是类）......但哪些取决于它们在做什么，它们有多复杂等等。通过将静态变量转换为实例变量，可以使作为类（无论出于何种原因）没有意义的状态“更加面向对象”。

Other Answers 建议具体重构，摆脱 all 全局变量，使用依赖注入等。我的看法是，没有足够的信息来判断这些建议是否会有所帮助。

但这值得吗？

仅仅让应用程序“更加面向对象”并不是一个有用或值得的目标。您的目标应该是使代码更具可读性、可维护性、可测试性、可重用性等。使用 OO 是否会改善问题取决于代码的当前状态，以及新设计和重构工作的质量。简单地采用 OO 实践不会纠正一个糟糕的设计，或者将“坏”代码变成“好”代码。

【讨论】：

【解决方案4】：

我将采取不同的方法来批评这一点，而不是它是否“过于程序化”。我希望你觉得它有些用处。

首先我没有看到任何函数参数或返回值。这意味着您可能正在使用各种全局数据，出于许多充分的理由应该避免使用这些数据，如果您愿意，可以在此处阅读：Are global variables bad?

其次，我没有看到任何错误检查逻辑。假设 resumeY 因异常而失败，可能问题出在 resumeY 中，但它也可能在 pauseY 中更高或与 loadDataSources 一样高，并且问题仅在稍后表现为异常。

我不知道这是否是生产代码，但它是在多个阶段进行重构的良好候选者。在第一阶段，您可以检查每个函数是否返回布尔值成功与否，并在每个函数的主体中检查已知错误情况。在您进行一些错误检查后，开始通过传入函数 args 并返回结果数据来删除您的全局数据；你可以让你的函数在失败的情况下返回空值或转换为异常处理，我建议异常。之后考虑使各个部分可单独测试；例如因此您可以将 downloadFeed 与数据处理功能分开进行测试，反之亦然。

如果您进行一些重构，您将开始看到可以模块化和改进代码的明显地方。 IMO，您应该少担心您是否足够 OOP，而应该多担心您是否可以 1. 有效地调试它，2. 有效地测试它和 3. 不理会它并在 6 个月后回来维护它后理解它.

这个回复很长，我希望你发现它的部分有用。 :-)

【讨论】：

【解决方案5】：

是的。您的方法不返回任何值，因此从 sn-p 看来它们正在对全局变量进行操作。它看起来像教科书的过程编程。

在更面向对象的方法中，我希望看到这样的内容：

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap()))
{
  MyDatasources ds = loadDataSources();
  Engine eng = initEngine(ds);
  DataObj data = loadLiveData(eng);
  Id[] xIds = processX(data.getX());
  Id[] newXIds = xIds.clone();
  data.addX(newXIds);
  Id[] yIds = processY(data.getY());
  Id[] newYIds = yIds.clone();
  data.addY(newYIds);
  pauseY();
  resumeY();
  someObject.setParameters(data);
}

保罗

【讨论】：

我不确定这里的 sn-p 是否更“面向对象”，但它显然优于原始版本。并且也优于大多数其他答案，它们试图通过将全局变量混乱隐藏在更多类后面来“解决”问题。

【解决方案6】：

这是我学会区分的方法：

当一个类有多个职责时，您的代码正在“程序化”的迹象（请参阅Single Responsibility Principle 上的此链接）。看起来所有这些方法都被一个对象调用，这意味着一个类正在管理一堆职责。

更好的方法是将这些职责分配给能够最好地处理它们的真实对象。一旦正确委派了这些职责，就可以实现能够有效驱动这些功能的软件模式（例如外观模式）。

【讨论】：

【解决方案7】：

这确实是过程式编程。如果你想让它更面向对象，我会尝试以下组合：

尝试让类代表您持有的数据。例如，有一个 FeedReader 类来处理提要，一个 DataLoader 类来加载数据等。
尝试将方法分成具有内聚功能的类。例如，将 resume()、pause() 组合到之前的 FeedReader 类中。
将 process() 方法分组到 ProcessManager 类中。

基本上，试着把你的计划想象成有一群员工为你工作，你需要为他们分配职责。（PM 为我做这件事，然后 DM 做这件事的结果）。如果你把所有的责任都交给一名员工，他或她就会精疲力尽。

【讨论】：

【解决方案8】：

我同意它看起来过于程序化。一种显而易见的方法使它看起来不那么程序化，就是让所有这些方法成为类的一部分。 Erhan 的帖子应该让您更好地了解如何分解它:-)

【讨论】：