一个方法应该承担多少责任?

3

这绝对是一个与语言无关的问题,而且已经困扰我相当长时间了。一个例子可能会帮助我解释我所面临的困境:

假设我们有一个方法,负责读取文件,用一些对象填充集合(这些对象存储来自文件的信息),然后返回该集合...就像下面的代码:

public List<SomeObject> loadConfiguration(String filename);

让我们假设在实现此方法时,如果返回的集合为空(大小为0),则应用程序继续似乎是不可行的。现在的问题是,这个验证(检查空集合并可能随后抛出异常)应该在方法内部完成吗?或者,这个方法的唯一责任应该是执行文件加载并忽略验证任务,允许在稍后的某个阶段完成验证?
我想一般的问题是:将验证与方法执行的实际任务解耦是否更好?这会使事情在以后更容易改变或建立 - 在我的上面的例子中,以后可能会添加不同的策略来恢复从“loadConfiguration”方法返回空集合的事件......如果在方法中进行验证(和结果异常),这将是困难的。
也许我在寻找一些教条式的答案时过于苛求了,在某种程度上只取决于方法使用的上下文。无论如何,我很想看看其他人对此有什么看法。
谢谢大家!
8个回答

7
我的建议是遵循单一职责原则,简而言之,每个对象应该只有一个目的。在这种情况下,您的方法有3个目的,如果计算验证方面,则有4个目的。
以下是我对如何处理此问题以及如何为未来的更新提供大量灵活性的建议:
1. 保留您的LoadConfig方法。 2. 调用一个新方法来读取文件。 3. 将上一个方法的返回值传递给另一个方法,以将文件加载到集合中。 4. 将对象集合传递到某个验证方法中。 5. 返回集合。
这将把最初的1个方法分成4个方法,其中一个调用其他3个方法。这样做应该可以让您更改某些部分而不会影响其他部分。
希望这可以帮助到您。

我理解这个建议背后的良好意图,但它有可能引发过度工程化的风险。 - MaD70
@MaD70 在某种程度上我同意你的看法;然而,如果要使用单元测试,将事物保持小而简单将极大地帮助保持代码可维护性。如果它只是一个小系统,那么我同意你的观点;然而,基于我的经验,最好总是将它们抽象成单一职责,否则间接错误的风险会增加。 - JamesEggers

4
我猜这个一般性问题是:将验证从方法实际执行的任务中解耦是否更好?
是的。如果你真的坚持回答这样一个一般性问题的话(很容易找到反例),将解决方案的两个部分分开,您可以交换、放弃或重复使用它们。这是一个明显的优点。当然,您必须小心,不要通过暴露非验证API来危及对象的不变量,但我认为您已经意识到了这一点。您将需要多做一些额外的输入工作,但这不会伤害您。

我们的看法不同。我认为,延迟验证(对参数、返回值等)是调试噩梦的一个开放大门。 - Matthieu M.
欢迎您不同意 :) 跳过单元测试是通往调试噩梦的大门。 如果您不刷掉设计,将保持危险线路的封装和外部 API 的测试,则无需担心。当然,我不会以我的生命为代价来维护这一点,显然有些情况下最好将加载和验证放在一起。 - zoul
我同意。一个反例:你不想被异常打扰,因为程序需要使用一些默认值开始某些选项,而其余的选项很少,所以你初始化它们所有,当没有配置文件(或为空)时不做任何事情,然后忘记它。 - MaD70
澄清:我的反例实际上支持了zoul的立场。 - MaD70

2
为了将问题转向更基础的问题,每种方法都应该尽可能做得越少越好。所以在您的例子中,应该有一个读取文件的方法,一个从文件中提取必要数据的方法,另一个将该数据写入集合的方法,以及调用这些方法的另一个方法。验证可以放在单独的方法中,也可以放在其他方法中,具体取决于哪个位置最合理。
   private byte[] ReadFile(string fileSpec)
   {  
       // code to read in file, and return contents
   }
   private FileData GetFileData(string fileContents)
   {
       // code to create FileData struct from file contents
   }
   private void FileDataCollection: Collection<FileData> { }

   public void DoItAll (string fileSpec, FileDataCollection filDtaCol)
   {
        filDtaCol.Add(GetFileData(ReadFile(fileSpec)));
   } 

针对每个方法,根据需要添加验证和确认步骤。


一个“基本”的方法应该尽可能少地做事情,但是一个方法聚合3/4个其他方法也是可以的。 - Matthieu M.

2
我将用一个问题来回答你的问题:你想为你的方法的产品使用各种验证方法吗?
这就像“构造函数”问题一样:在构建过程中引发异常是否更好,还是初始化一个空对象然后调用“init”方法……你肯定会在这里引发一场辩论!
一般来说,我建议尽早进行验证:这被称为快速失败,它主张尽早发现问题比延迟检测要好,因为诊断是立即的,而稍后你将不得不恢复整个流程……
如果你还没有被说服,请这样考虑:每次加载文件时你真的想写3行代码吗?(加载、解析、验证)那违反了DRY原则。
所以,采用敏捷方式:
  • 编写带有验证的方法:它负责加载一个有效的配置(1)
  • 如果你需要某些参数化,请添加它(例如一个“check”参数,具有默认值,当然也保留了旧的行为)
当然,我不主张一种方法来一次完成所有这些... 这是一个组织问题:在幕后,这种方法应该调用专门的方法来组织代码 :)

我给你点赞,尽管我们的答案不同。我认为我们的意思是一样的:你说要将验证与加载放在一起,但在内部方法中分开实现;我说要保持它们分开,但在外部 API 中暴露部分时要小心。 - zoul
一般而言,我建议......但是并不存在“通用程序”,只有特定的程序,由特定的人编写,解决特定的问题(无论多么通用,程序都不能解决存在的每个问题)。请参见我对zoul的回复,其中举例说明在空(或不存在)配置文件的情况下不想引发异常。 - MaD70
我不明白你的观点...当然,如果有情况下你对某种情况感到满意,那么就不要将其视为错误。然而,不处理任何错误是一种懒惰的方法,你必须仔细定义你的方法的契约,并在需要重构时进行演化。 - Matthieu M.

2
您正在设计一个API,不应该对客户端做出任何不必要的假设。一个方法应该只获取它所需要的信息,返回请求的信息,并且只有在无法返回有意义的值时才会失败。
因此,在这种情况下,如果配置是可加载但为空,则返回空列表似乎是正确的。如果您的客户端对提供空列表时失败具有特定应用程序要求,则可以这样做,但未来的客户端可能没有这个要求。loadConfiguration方法本身应该在真正失败时失败,例如当它无法读取或解析文件时。
但是您可以继续解耦您的接口。例如,为什么配置必须存储在文件中?为什么我不能提供URL、数据库中的一行或包含配置数据的原始字符串?很少有方法应该将文件路径作为参数,因为它们将它们紧密地绑定到本地文件系统,并使它们负责打开、读取和关闭文件以及它们的核心逻辑。考虑接受输入流作为替代方案。或者如果您想允许精细的替代方案--比如从数据库中获取数据--请考虑接受ConfigurationReader接口或类似接口。

0

方法应该高度内聚...也就是单一职责。因此,我的观点是按照您所描述的分开职责。有时我会感到诱惑,说...这只是一个简短的方法,所以并不重要...然后1.5周后我会后悔。


0

我认为这取决于情况:如果你能想到一个场景,在这个场景中你会使用这种方法并且返回一个空列表是可以接受的,那么我不会在方法内部放置验证。但对于例如向数据库插入必须进行验证的数据的方法(电子邮件地址是否正确、是否指定了名称等),应该将验证代码放在函数内部并抛出异常。


0
另一种未被提及的选择是支持依赖注入,并让方法客户端注入验证器。这将允许保留“强”资源获取即初始化原则,即成功加载的任何对象都准备好进行业务操作(Matthieu提到的快速失败是同样的概念)。
它还允许资源实现类创建其自己的低级别验证器,这些验证器依赖于资源的结构,而不会不必要地暴露客户端的实现细节,在处理多个不同的资源提供程序时可能非常有用,如Ryan所列举的。

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接