Java私有方法签名清晰和更好的实践[关闭]

Question

我收到以下代码 sn-p 的代码审查评论 -

public void doWork(String a, Integer b) {
    ..
    ..
    SomeService service = getService();
    for (Integer i : numbers) {
      doMoreWork(a, b, service);
    }
}
private void doMoreWork(String a, Integer b, SomeService service) {
    ...
    ...
    ...
    service.doingMoreWork(a, b);
}

审查建议 -

1. SomeService service = getService(); 应在 doMoreWork 内调用，以便签名清晰。所以签名变成了doMoreWork(a, b)，这更容易理解。

我的问题 -

由于doMoreWork() 在循环中发生，我将一个服务对象传递给它。 doMoreWork() 只是代码中的一个私有逻辑单元，将“服务”作为方法参数携带应该没问题。这种方法永远不会公开。这种情况下的指导方针是什么？此处的清晰度或可读性如何受到影响？

（注意：

1. getService() 调用的性能开销并不大，因此这里的性能不是我的标准。
2. 这里不提供服务注入选项。需要按照示例中显示的方式获取。
3. doMoreWork() 不仅仅是调用 service.doingMoreWork()。它有一些要遵循的前置步骤。

)

Answer 1

如果您将doMoreWork() 重构为以下内容：

private void doMoreWork(String a, Integer b) {
    SomeService service = getService();
    service.doingMoreWork(a, b);
}

那么您正在使 doMoreWork() 依赖于 getService() 的功能。如果您想使用不同的方法来获取SomeService 对象，则必须重构此代码。因此，出于这个原因，您可能希望坚持使用原始实现。

如果您使用像 Spring 这样的框架，您可以像这样将 SomeService 注入到您的类中：

@Autowired
private SomeService service;

// use the injected service here
private void doMoreWork(String a, Integer b) {
    service.doingMoreWork(a, b);
}

Answer 2

建议是正确的，您仅在 doMoreWork 方法中需要 Service 对象。所以最好只在doMoreWork方法中获取Service对象来执行你的操作。

如果在loop之后添加更多语句，那么Service对象将持续很长时间，直到它的范围在那里（直到没有遇到关闭}），它不会被垃圾收集因此你应该调用@ doMoreWork 中的 987654328@ 并在 doMoreWork(a,b) 中传递 2 个参数。会更容易理解。

Answer 3

从代码清晰的角度来看，doMoreWork 是一个知道其服务的类中的实例方法（即，它有一个getService() 方法）。将Service 放在签名中会让读者想知道Service 这个方法指的是什么，所以有点混乱。不过，在一个具有多个服务并且必须对其中几个服务执行相同类型的工作的类中，这样的签名非常有意义。

Answer 4

老实说，除了在doMoreWork 方法中声明该变量并没有什么好处。原因如下：

• 在doWork 内部没有对其进行（明显）突变
• getService() 似乎总是返回相同的 SomeService 实例

取决于getService 是否只是一个字段的吸气剂，那么直接使用该字段而不是通过该方法可能会更好。但是，您确实不需要将此值传递给方法。

Answer 5

我同意代码审查，getService() 应该在doMoreWork()。

另外，我建议将服务作为类的依赖项，也就是说，服务可以是类的一个字段。一些 DI 框架可以为您注入服务，例如 Spring 框架。

如果是这样，this.service 可以在您的类的每个方法中引用。

Answer 6

如果getService() 每次都返回一个新实例，但它的名称应该改为createNewService()，则将服务作为参数传递是有意义的。

在当前情况下，getService() 似乎总是返回相同的实例，因此将 Service 作为参数传递并没有任何区别，只会让读者感到困惑。

进一步补充，getService() 是在同一个类中定义的，doMoreWork() 是一个私有方法，所以它可能总是引用同一个服务，而不是为什么每次都添加一个额外的步骤来传递Service！

另一件事，如果doMoreWork() 所做的只是调用service.doingMoreWork(a, b);，那么最好删除此方法并直接在循环中调用它，如下所示：

public void doWork(String a, Integer b) {
    ..
    ..
    for (Integer i : numbers) {
        getService().doingMoreWork(a, b);
    }
}

Answer 7

没关系，因为：

doMoreWork() 标记为private，这是该类的隐藏细节。 由于它不是public，它不是public API 或合同的一部分，它是一个可以更改的实现细节。

因此，您的问题中给出的原因是什么签名并不重要，这只是个人喜好。

您永远不会在循环中创建新的 JDBC 连接，这将是非常低效的，即使有一个好的连接池也是错误的做法。

所有这些都说我会这样做。

private 方法不是必需的，只会增加混乱和意见。

public void doWork(@Nonnull final String s, @Nonnull final Integer i) 
{
    final SomeService service = getService();
    for (final Integer n : numbers) 
    {
      service.doingMoreWork(s, i);
    }
}