3

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

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

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

另外 10 行根据子域是否在请求中填充一些视图变量。

一个 10 行的部分，根据以下内容重定向到两个页面之一：

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

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

另外 10 行来构建动态 arel。

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

4

3 回答 3

1

这是设置了很多变量。也许这是某种模块的好机会？也许您的模块可以为您做出很多这样的决定，以及充当很多这些变量的包装器。抱歉，我没有更具体的答案。

于 2010-10-26T14:56:53.867 回答
1

用类似代码的方式总结你所说的（对不起，不知道 ruby​​；认为它是伪代码）：

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

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

于 2010-10-27T13:47:49.650 回答
1

如果没有您的代码，建议实际修复有点困难，但这听起来绝对是一种非常错误的方法，并且您正在使事情变得比他们需要的更难：

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

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

另外 10 行根据子域是否在请求中填充一些视图变量。

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

一个 10 行的部分，根据以下内容重定向到两个页面之一：

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

另外 10 行来构建动态 arel。

可能有必要进入 Arel，但我高度怀疑它。在大多数情况下，您的实际搜索调用可以（并且应该是）1 行，通过标准的 ActiveRecord 查询接口完成。你想在你的模型中设置强大的范围，通过 ActiveRecord 查询接口来处理加入其他模型/缩小条件等。

于 2011-06-19T17:45:05.513 回答