这个 Rails 3 Controller 方法会让我看起来很胖吗？

Question

这是一个新的应用程序，我在搜索控制器上有一个索引方法。这也作为应用程序的主页，我试图从设计模式的角度确定我是否走上了错误的道路。

该方法已经有 35 行长。该方法的作用如下：

3 行设置变量来确定正在搜索的分层数据的“级别”。

另外 10 行用于根据请求中是否包含子域来填充一些视图变量。

重定向到两个页面之一的 10 行部分，基于：

1) 如果用户没有访问权限，但已登录，并且尚未请求访问权限，请告诉他们“单击此处请求访问该品牌”。

2) 如果用户没有访问权限、已登录且已请求访问权限，请告诉他们“某某正在审核您的请求”。

另外 10 条线用于构建动态区域。

我无法弄清楚如何区分这些问题，或者即使它们应该分开。我感谢您提供的任何帮助！

Answer 1

总结一下你在类似代码中所说的内容（抱歉，不了解 ruby​​；将其视为伪代码）：

void index() {
  establishHierarchyLevel();
  if (requestIncludedSubdomain())
    fillSubdomainFields();
  else
    fillNonsubdomainFields();
  if (user.isSignedIn() && !user.hasAccess()) {
    if (user.hasRequestedAccess())
      letUserIn();
    else
      adviseUserOfRequestUnderReview();
  }
  buildDynamicArelWhateverThatIs();
}

14 行而不是 35 行（当然，提取方法的主体会延长整体代码，但你可以看看这个并知道它在做什么）。值得做吗？这实际上取决于您或后续程序员是否更清楚。我的猜测是值得这样做，将小代码块拆分为自己的方法将使代码更易于维护。

Answer 2

设置了很多变量。也许这对于某种模块来说是一个很好的机会？也许你的模块可以为你做出很多这样的决定，并且充当很多这样的变量的包装器。抱歉我没有更具体的答案。

Answer 3

如果没有您的代码，就很难提出实际的修复建议，但这听起来绝对是一种真正错误的方法，并且您使事情变得比需要的更加困难：

3行设置变量
确定层次结构的“级别”
正在搜索数据

，我认为您会希望将这些数据直接从 params 哈希传递到范围或 Model.where() 调用中。根据需要在模型上设置范围。

另外 10 行用于根据请求中是否包含子域来填充一些视图变量。

在我看来，它最多应该是 1 行。或者在您看来，您应该使用 if 语句来根据您的子域更改您希望的输出内容。

一个 10 行部分，用于重定向到两个页面之一，基于：

您对两个视图的解释中唯一不同的是“用户是否已请求访问”这肯定只是一个布尔变量？您只需要 1 个视图。将差异包装成 2 个部分，然后在您的视图中编写一个 if 语句以在它们之间进行选择。

另外 10 行用于构建动态区域。

可能有必要进入阿雷尔，但我非常怀疑。在大多数情况下，您的实际搜索调用可以（并且应该是）1 行，通过标准 ActiveRecord 查询接口完成。您希望在模型中设置强大的范围，以通过 ActiveRecord 查询接口连接到其他模型/缩小条件等。