12

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

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

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

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

非常感谢所有评论！

4

8 回答 8

8

好吧，这段代码所在的类显然有太多的责任。我不会将所有这些东西隐藏在外观中，而是将所有与某些 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 规则。

于 2011-05-13T23:04:59.003 回答
6

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

无法说此代码片段是否“过于程序化”。

  • 这些调用都可能是对当前对象的实例方法，对单独的对象或当前实例的实例变量进行操作。至少在某种程度上，这些将使代码 OO。

  • 如果这些方法是static，那么代码确实是程序性的。这是否是一件坏事取决于方法是否正在访问和更新存储在static字段中的状态。（从名字来看，他们可能需要这样做。）

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

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

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

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

其他答案建议特定的重构，摆脱所有全局变量，使用依赖注入等等。我的看法是，没有足够的信息来判断这些建议是否会有所帮助。

但是这值得吗？

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

于 2011-05-14T01:22:47.000 回答
6

哇 ！它看起来不像 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

于 2011-05-13T22:47:07.260 回答
4

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

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

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

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

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

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

于 2011-05-14T04:11:02.987 回答
3

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

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

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);
}

保罗

于 2011-05-13T22:55:00.180 回答
2

以下是我学会区分的方法：

当一个类具有多个职责时，您的代码正在变得“程序化”的标志（参见单一职责原则上的此链接）。看起来所有这些方法都被一个对象调用，这意味着一个类正在管理一堆职责。

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

于 2011-05-13T22:56:55.983 回答
1

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

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

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

于 2011-05-13T23:00:37.147 回答
0

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

于 2011-05-13T22:48:47.710 回答