Этот метод Rails 3 Controller заставляет меня выглядеть толстым? - PullRequest
3 голосов
/ 26 октября 2010

Это новое приложение, и у меня есть метод индекса на контроллере поиска. Это также служит домашней страницей для приложения, и я пытаюсь решить, иду ли я по неверному пути с точки зрения шаблона проектирования.

Метод уже 35 строк. Вот что делает метод:

3 строки установки переменных для определения того, какой «уровень» иерархических данных ищется.

Еще 10 строк для заполнения некоторых переменных представления в зависимости от того, был ли субдомен в запросе или нет.

10-строчный раздел для перенаправления на одну из двух страниц на основе:

1) Если у пользователя нет доступа, он вошел в систему и еще не запросил доступ, скажите ему «нажмите здесь, чтобы запросить доступ к этой марке».

2) Если пользователь не имеет доступа, выполнил вход и уже запросил доступ, скажите ему, что "ваш запрос рассматривается таким же образом".

Еще 10 строк для построения динамического арла.

Я не могу понять, как разделить эти проблемы, или даже если они должны быть разделены. Я ценю любую помощь, которую вы можете предложить!

Ответы [ 3 ]

1 голос
/ 27 октября 2010

Обобщение того, что вы сказали, в чем-то похожем на код (извините, не знаю ruby; считайте это псевдокодом):

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

14 строк вместо 35 (конечно, тела извлеченных методовбудет удлинять весь код, но вы можете посмотреть на это и узнать, что он делает).Стоит ли это делать?Это действительно зависит от того, понятнее ли это вам или последующим программистам.Полагаю, стоит сделать так, чтобы разбиение небольших блоков кода на их собственный метод облегчало поддержку кода.

1 голос
/ 19 июня 2011

Без вашего кода довольно сложно предложить реальные исправления, но это определенно звучит как действительно неправильный подход, и вы делаете вещи намного сложнее, чем они должны быть:

3 строки установки переменных в определить, какой "уровень" иерархического данные ищутся

если есть форма поиска, я думаю, вы захотите передать ее прямо из хэша params в scopes или вызовы Model.where (). Настройте области действия на вашей модели соответствующим образом.

Еще 10 строк для заполнения некоторых переменных представления в зависимости от того, был ли субдомен в запросе или нет.

Мне кажется, это должно быть не более 1 строки. или что, по вашему мнению, вы должны использовать операторы if, чтобы изменить то, что вы хотите, чтобы ваш вывод зависел от вашего субдомена.

10-строчный раздел для перенаправления на одну из двух страниц на основе:

единственное, что отличается в вашем объяснении двух представлений, это «запросил ли пользователь доступ», конечно, это просто логическая переменная? Вам нужен только 1 просмотр. Оберните различия в 2 части, а затем, на ваш взгляд, и напишите одно из них, чтобы выбрать между ними.

Еще 10 строк для построения динамического арла.

* * * * * * * * * * * * * * * * * * * * * мог бы быть нуждаться в 1028 *, чтобы войти в Арель, но я очень сильно сомневаюсь в этом. Ваш фактический поисковый вызов может в большинстве случаев (и должен стремиться быть) одной строкой, сделанной через стандартный интерфейс запросов ActiveRecord. Вы хотите настроить в своих моделях сильные области, которые заботятся о присоединении к другим моделям / сужающим условиям и т. Д. Через интерфейс запроса ActiveRecord.

1 голос
/ 26 октября 2010

Это много переменных, которые устанавливаются. Может быть, это хорошая возможность для какого-то модуля? Возможно, ваш модуль может принять многие из этих решений за вас, а также выступить в качестве оболочки для многих из этих переменных. Извините, у меня нет более конкретного ответа.

...