Как справиться с тривиальным запахом «дублирования кода» в Rails / Ruby - PullRequest
Новая
       20

Как справиться с тривиальным запахом «дублирования кода» в Rails / Ruby

5 голосов
/ 20 февраля 2010

Итак, мы все стремимся уменьшить дублирование (DRY) и другие неприятные запахи, и сделать наш код максимально красивым и чистым. Для кода на Ruby существует множество инструментов для обнаружения запахов, например, довольно приятный сервис Caliber .

Однако, похоже, что у меня есть другое определение дублирования кода, чем у инструментов. Я думаю, что это может быть связано с тем, как работает Ruby, когда вы почти никогда не обращаетесь к переменной напрямую, а через вызов метода. Рассмотрим этот фрагмент из контроллера Rails: 

def update_site_settings
  SiteSettings.site_name = params[:site_name]
  SiteSettings.site_theme = params[:site_theme]
  expire_fragment('layout_header')
  flash[:notice] = t(:Site_settings_updated)
  redirect_to :controller => 'application', :action => 'edit_site_settings'
end

Это помечено предупреждением о дублировании кода из-за двух вызовов метода "params". Итак, мой вопрос: действительно ли было бы лучше назначить params локальной переменной? Я считаю, что способ написания этой статьи является наиболее понятным и лаконичным способом сделать это, а тот факт, что params - это метод, а не переменная, представляет собой простую «стоимость ведения бизнеса» в Ruby.

Я вижу это неправильно?

РЕДАКТИРОВАТЬ: В этом случае, более красивым способом может быть обновление стиля SiteSettings.update_attributes(params). Рассмотрим, если хотите, ту же проблему в другом фрагменте: 

def update
  @mailing_list = MailingList.find(params[:id])

  if @mailing_list.update_attributes(params[:mailing_list])
    flash[:notice] = t:Mailing_list_updated
    redirect_to(mailing_lists_path)
    ...

Ответы [ 2 ]

3 голосов
/ 20 февраля 2010

Следует помнить о понятиях «СУХОЙ» и «запах кода»: они являются рекомендациями . Они помогут вам подумать о том, как упорядочить и упростить код в более широком понимании. Хотя полезно всегда помнить об этих концепциях, придирчивость к повторению кода на таком маленьком уровне часто приводит к тому, что в вашем коде появляются ненужные сложности или неясность. Понятность вашего кода также должна быть важна, и, как вы говорите, иногда наиболее понятный и лаконичный код не совпадает с кодом, где удаляются все последние следы повторения.

1 голос
/ 20 февраля 2010

Полагаю, вы также можете объявить второстепенный экземпляр Feature Envy запахом кода, хотя это действительно несколько тривиально.

Возможно, можно добавить метод класса в MailingList, поэтому метод контроллера станет примерно таким: 

def update
  if @mailing_list = MailingList.update_attributes_by_id(params)
    ...

class MailingList
  def self.update_attributes_by_id(params)
     id = params.delete(:id)
     find(id).update_attributes(params)
     ...

(не проверено, поэтому обращайтесь с осторожностью)

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

Эти анализаторы (я запускаю reek Кевина Резерфорда на моем собственном коде) великолепны, но они не понимают контекст, поэтому редко предоставляют идеальную информацию. Они полезны для определения областей, которые могут извлечь выгоду из внимания, но будет много ложных срабатываний, и они также будут пропускать вещи, поэтому их нужно использовать с осознанием.

...