6

I've organized my code hierarchically and I find myself crawling up the tree using code like the following.

File clientFolder = task.getActionPlan().getClientFile().getClient().getDocumentsFolder();

I'm not drilling down into the task object; I'm drilling up to its parents, so I don't think I'm losing anything in terms of encapsulation; but a flag is going off in the back of my mind telling me there's something dirty about doing it this way.

Is this wrong?

4

20 回答 20

5

旗帜是红色的,它用粗体表示两件事:

  • 为了遵循链,调用代码必须知道整个树结构,这不是很好的封装,并且
  • 如果层次结构发生变化,您将需要编辑大量代码

括号中的一件事:

  • 使用属性,即 task.ActionPlan 而不是 task.getActionPlan()

一个更好的解决方案可能是 - 假设您需要在子级别的树上公开所有父属性 - 继续并在子级上实现直接属性,即

File clientFolder = task.DocumentsFolder;

这至少会从调用代码中隐藏树结构。在内部,属性可能如下所示:

class Task {
    public File DocumentsFolder {
        get { return ActionPlan.DocumentsFolder; }
    }
    ...
}
class ActionPlan {
    public File DocumentsFolder {
        get { return ClientFile.DocumentsFolder: }
    }
    ...
}
class ClientFile {
    public File DocumentsFolder {
        get { return Client.DocumentsFolder; }
    }
    ...
}
class Client {
    public File DocumentsFolder {
        get { return ...; } //whatever it really is
    }
    ...
}

但是如果将来树结构发生变化,您只需要更改树中涉及的类中的访问器函数,而不是您调用链的每个地方。

[另外,在属性函数中正确地捕获和报告空值会更容易,这在上面的示例中被省略了]

于 2008-10-01T21:05:54.340 回答
4

Well, every indirection adds one point where it could go wrong.

In particular, any of the methods in this chain could return a null (in theory, in your case you might have methods that cannot possibly do that), and when that happens you'll know it happened to one of those methods, but not which one.

So if there is any chance any of the methods could return a null, I'd at least split the chain at those points, and store in intermediate variables, and break it up into individual lines, so that a crash report would give me a line number to look at.

Apart from that I can't see any obvious problems with it. If you have, or can make, guarantees that the null-reference won't be a problem, it would do what you want.

What about readability? Would it be clearer if you added named variables? Always write code like you intend it to be read by a fellow programmer, and only incidentally be interpreted by a compiler.

In this case I would have to read the chain of method calls and figure out... ok, it gets a document, it's the document of a client, the client is coming from a ... file... right, and the file is from an action plan, etc. Long chains might make it less readable than, say, this:

ActionPlan taskPlan = task.GetActionPlan();
ClientFile clientFileOfTaskPlan = taskPlan.GetClientFile();
Client clientOfTaskPlan = clientFileOfTaskPlan.GetClient();
File clientFolder = clientOfTaskPlan.getDocumentsFolder();

I guess it comes down to personal opinion on this matter.

于 2008-10-01T20:46:29.053 回答
3

getter 和 setter 是邪恶的。一般来说,避免让一个对象用它做某事。而是委派任务本身。

代替

Object a = b.getA();
doSomething(a);

b.doSomething();

与所有设计原则一样,不要盲目地遵循这一点。如果没有 getter 和 setter,我永远无法编写任何远程复杂的东西,但这是一个很好的指导方针。如果你有很多 getter 和 setter,这可能意味着你做错了。

于 2008-10-01T23:04:12.233 回答
2

首先,像这样堆叠代码会使在单步调试器中分析 NullPointerExceptions 和检查引用变得很烦人。

除此之外,我认为这一切都归结为:调用者需要具备所有这些知识吗?

也许它的功能可以变得更通用;然后可以将 File 作为参数传递。或者,也许 ActionPlan 甚至不应该透露它的实现是基于 ClientFile 的?

于 2008-10-01T20:53:24.543 回答
1

我同意提到得墨忒耳法则的海报。您正在做的是对许多这些类的实现以及层次结构本身的结构创建不必要的依赖关系。这将使孤立地测试您的代码变得非常困难,因为您需要初始化十几个其他对象才能获得要测试的类的工作实例。

于 2008-10-01T20:59:05.573 回答
1

多么及时。今晚我将在我的博客上写一篇关于这种气味Message Chains与其相反的Middle Man的文章。

无论如何,一个更深层次的问题是为什么你对似乎是域对象的东西有“get”方法。如果您密切关注问题的轮廓,您会发现告诉任务获取某些东西是没有意义的,或者您正在做的实际上是一个非业务逻辑问题,例如准备 UI 显示,持久化,对象重建等。

在后一种情况下,只要授权类使用“get”方法就可以了。您如何执行该策略取决于平台和流程。

所以在“get”方法被认为没问题的情况下,你仍然必须面对这个问题。不幸的是,我认为这取决于在链条中导航的类。如果该类与结构(例如,工厂)耦合是合适的,那就顺其自然吧。否则,您应该尝试隐藏委托

编辑:点击这里查看我的帖子

于 2008-10-01T21:25:27.523 回答
1

你真的会独立使用这些功能中的每一个吗?为什么不让任务拥有一个 GetDocumentsFolder() 方法来为您完成调用所有这些方法的所有脏活?然后,您可以让它完成所有对所有内容进行空值检查的肮脏工作,而无需在不需要修饰的地方修饰您的代码。

于 2008-10-01T23:37:34.257 回答
0

The biggest flag in the world.

You cannot check easily if any of those invokations returns a null object thus making tracking any sort of error next to impossible!

getClientFile() may return null and then getClient() will fail and when you are catching this, assuming you are try-catching you won't have a clue as to which one failed.

于 2008-10-01T20:43:51.040 回答
0

How likely is it to get nulls or invalid results? That code is dependent on the successful return of many functions and it could be harder to sort out errors like null pointer exception.

It's also bad for the debugger: less informative since you have to run the functions rather than just watching a few local variables, and awkward to step into the later functions in the chain.

于 2008-10-01T20:44:34.720 回答
0

Yes. It's not best practice. For one thing, if there's a bug, it's harder to find it. For example, your exception handler might display a stack trace that shows that you have a NullReferenceException on line 121, but which of these methods is returning null? You'd have to dig into the code to find out.

于 2008-10-01T20:44:35.343 回答
0

This is a subjective question but I don't think there's anything wrong with it up to a point. For instance if the chain extends beyond the readable aread of the editor then you should introduce some locals. For instance, on my browser I can't see the last 3 calls so I have no idea what you're doing :).

于 2008-10-01T20:45:31.597 回答
0

Well it depends. You shouldn't have to reach through an object like that. If you control the implementations of those methods, I'd recommend refactoring so that you don't have to do that.

Otherwise, I see no harm in doing what you're doing. It's certainly better than

ActionPlan AP = task.getActionPlan();
ClientFile CF = AP.getClientFile();
Client C = CF.getClient();
DocFolder DF = C.getDocumentsFolder();
于 2008-10-01T20:45:47.400 回答
0

这样还不错,但是您可能会在 6 个月内阅读此内容时遇到问题。或者同事可能会在维护/编写代码时遇到问题,因为您的对象链非常……长。

而且我认为您不必在代码中引入变量。所以对象确实知道从一个方法跳转到另一个方法所需的一切。(这里出现了一个问题,如果你没有过度设计一点点,但我该告诉谁?)

我将介绍一种“便利方法”。想象一下,您的“任务”中有一个方法 - 对象类似于

    task.getClientFromActionPlan();

那么你肯定可以使用

    task.getClientFromActionPlan().getDocumentsFolder();

更具可读性,如果您正确执行这些“便利方法”(即对于重度使用的对象链),则键入的更少;-)。

Edith 说:我建议的这些便捷方法在我编写它们时通常确实包含 Nullpointer-checking。这样,您甚至可以在其中抛出带有良好错误消息的 Nullpointers(即“尝试从中检索客户端时 ActionPlan 为空”)。

于 2008-10-01T20:46:56.493 回答
0

相关讨论:

函数链 - 多少算太多?

于 2008-10-01T20:49:10.980 回答
0

这个问题非常接近https://stackoverflow.com/questions/154864/function-chaining-how-many-is-too-many#155407

总的来说,似乎人们同意太长的链不好,你应该最多坚持一两个链式调用。

虽然我听说 Python 爱好者认为链接很有趣。这可能只是一个谣言......:-)

于 2008-10-01T20:51:55.217 回答
0

根据您的最终目标,您可能希望使用 The Principal of Least Knowledge 来避免严重的耦合并最终使您付出代价。正如 head first 喜欢说的那样.. “只和你的朋友说话。”

于 2008-10-01T20:57:12.617 回答
0

链接的另一个重要副产品是性能。

在大多数情况下,这没什么大不了的,但尤其是在循环中,您可以通过减少间接性看到性能的合理提升。

链接也使估计性能变得更加困难,您无法判断哪些方法可能会或可能不会做一些复杂的事情。

于 2008-10-01T21:01:27.597 回答
0

好的,正如其他人指出的那样,代码不是很好,因为您将代码锁定到特定的层次结构。它可能会出现调试问题,而且阅读起来并不好,但主要的一点是执行任务的代码对遍历获取某些文件夹的内容了解太多。美元到甜甜圈,有人会想在中间插入一些东西。(所有任务都在任务列表中,等等)

冒险出去,所有这些类都只是同一事物的特殊名称吗?即它们是分层的,但每个级别可能有一些额外的属性?

所以,从不同的角度,我将简化为一个枚举和一个接口,如果子类不是被请求的东西,它们会在链上进行委托。为了争论,我称它们为文件夹。

enum FolderType { ActionPlan, ClientFile, Client, etc }

interface IFolder
{
    IFolder FindTypeViaParent( FolderType folderType )
}

并且每个实现 IFolder 的类可能只是

IFolder FindTypeViaParent( FolderType folderType )
{
    if( myFolderType == folderType )
        return this;

    if( parent == null )
        return null;

    IFolder parentIFolder = (IFolder)parent;
    return parentIFolder.FindTypeViaParent(folderType)
}

一种变体是制作 IFolder 接口:

interface IFolder
{
    FolderType FolderType { get; }
    IFolder Parent { get; }
}

这允许您将遍历代码外部化。然而,这将控制权从类(可能它们有多个父类)并公开实现。好和坏。

[杂谈]

乍一看,这似乎是一个非常昂贵的层次结构。我每次都需要自上而下实例化吗?即,如果某事只需要一项任务,您是否必须自下而上实例化所有内容以确保所有这些后向指针工作?即使它是延迟加载,我是否需要爬上层次结构才能获得根?

再说一遍,层次结构真的是对象身份的一部分吗?如果不是,也许您可​​以将层次结构外部化为 n 叉树。

作为旁注,您可能需要考虑聚合的 DDD(域驱动设计)概念并确定主要参与者是谁。负责的最终所有者对象是什么?例如汽车的轮子。在为汽车建模的设计中,车轮也是物体,但它们归汽车所有。

也许它对你有用,也许它不。就像我说的,这只是在黑暗中的一个镜头。

于 2008-10-02T12:36:26.430 回答
0

我也会指出得墨忒耳法则。

并添加一篇关于告诉,不要问的文章

于 2008-10-01T23:27:10.353 回答
0

另一方面,得墨忒耳法则不是普遍适用的,也不是硬性规则(arrr,它更像是一个指导方针!)。

于 2009-03-07T11:11:41.800 回答