Комментарии 19
На самом деле — отличный пример избыточного ООП
Этот код вполне неплох, несмотря на его недостатки. Имхо, необходимо приступать к его рефакторингу тогда, когда это реально понадобится и не плодить лишних сущностей раньше времени.
class Post < ActiveRecord::Base
def self.as_dictionary
dictionary = ('A'..'Z').inject({}) {|h, l| h[l] = []; h}
Post.all.each do |p|
dictionary[p.title[0]] << p
end
dictionary
end
end
Этот код вполне неплох, несмотря на его недостатки. Имхо, необходимо приступать к его рефакторингу тогда, когда это реально понадобится и не плодить лишних сущностей раньше времени.
Автор же говорит потом, что нужно отобразить не только посты, но и пользователей + отделение представления от логики. Так что вполне нормально.
Главное знать чувство меры)
Главное знать чувство меры)
Думаю, в оригианальном посте, он просто пытался напомнить, что лучше программировать «вне» фреймворка. Тогда «модель» (в широком смысле этого слова, не только «модель данных») получается чище, её при прочих равных — проще тестировать + имея такую модель проще менять интерфейсы: сегодня это будет веб-интерфейс с помощью ROR, завтра — stand-alone демон + некоторый RPC, а после завтра — десктопное приложение, или — «всё сразу».
Отличная фраза про «вне фреймворка». И это касается не только Руби, и не только Рельсов. POCO (PORO, POJO) в любом случае позволяют сделать код как можно более независимым от инфраструктуры.
Mark Seemann публикует в своем блоге статьи про ООП, и о том, как использовать его реальные преимущества. Хотя он и работает с .NET, все эти принципы применимы к любому ООП-языку.
Mark Seemann публикует в своем блоге статьи про ООП, и о том, как использовать его реальные преимущества. Хотя он и работает с .NET, все эти принципы применимы к любому ООП-языку.
В обоих примерах Клабник пишет что это всего лишь примеры, поэтому в первом просит не бросаться на Post.all, а во втором — не пытаться решить проблему первым способом (потому что это тоже правильно, но надо показать другое).
Во-первых, «Feather» на самом деле Физерс (потому что книга на русский переведена уже давно). А во-вторых, зачем ссылаться на практики работы с унаследованным кодом, когда код не унаследован, и можно спокойно использовать обычный рефакторинг?
(то есть мы понимаем, конечно, что Физерс просто ссылается на Фаулера, но, об этом, собственно, и речь — чего бы не пойти сразу к первоисточнику)
(то есть мы понимаем, конечно, что Физерс просто ссылается на Фаулера, но, об этом, собственно, и речь — чего бы не пойти сразу к первоисточнику)
Мне почему то кажется, что передавать Policy в примере с DictionaryPresenter не совсем «ruby-way».
Почему бы не педавать Block прямо в метод as_dictionary. Выглядело бы примерно так:
Почему бы не педавать Block прямо в метод as_dictionary. Выглядело бы примерно так:
DictionaryPresenter.new(Post.all).as_dictionary do |i|
if i.starts_with?("A ")
i.title.split[1][ 0]
else
i.title[ 0]
end
end
В блоге автора статьи есть продолжение, где он говорит и о таком варианте. Он имеет право на жизнь, но есть два недостатка:
1. Получается менее ясно (что именно делает этот блок? Объект Policy с хорошим именем проясняет этот момент).
2. Повторение кода. Если вдруг надо изменить политику — надо менять ее во всех местах в коде, где вы ее вызывали.
1. Получается менее ясно (что именно делает этот блок? Объект Policy с хорошим именем проясняет этот момент).
2. Повторение кода. Если вдруг надо изменить политику — надо менять ее во всех местах в коде, где вы ее вызывали.
Если в теле метода контроллера, то однозначно передавать Policy
НЛО прилетело и опубликовало эту надпись здесь
Это палка о двух концах.
Чаще всего такой подход превращается в божественные объекты и противоречит базовым принципам объектно-ориентированного дизайна.
Чаще всего такой подход превращается в божественные объекты и противоречит базовым принципам объектно-ориентированного дизайна.
Отлично! Люблю статьи, где мы DRYим и KISSим модели рельс. Взял инструмент на заметку.
о, конечно. и в каком месте мы тут что отDRYли или от KISSили?
Мы выносили повторяющуюся функциональность в отдельный класс. Избежание повторения кода соответствует концепции don't repeat yourself. А KISS как следствие того что код не превращается в помойку, а остается простым и понятным. Посмотрите скринкасты по рельсам, там такого вида рефакторинг применяется постоянно, как одна из основных концепций программирования на ruby/rails.
Автор молодец, яростно плюсую!
Just for fun:
def self.as_dictionary
letters = Hash[('A'..'Z').to_a.product([[]])] #U can skip this if u don't need empty letters
letters.merge self.all.group_by{|u| u.last_name[0..1] } # :)
end
Вообще без должного опыта и природного
чутья за объектную декомпозицию нужно браться очень осторожна.
Недоабстракция и оверабстракция не многим лучше семантического разрыва в процедурном стиле
def self.as_dictionary
letters = Hash[('A'..'Z').to_a.product([[]])] #U can skip this if u don't need empty letters
letters.merge self.all.group_by{|u| u.last_name[0..1] } # :)
end
Вообще без должного опыта и природного
чутья за объектную декомпозицию нужно браться очень осторожна.
Недоабстракция и оверабстракция не многим лучше семантического разрыва в процедурном стиле
С точностью до логики, не ООП рефакторинг другого примера:
выявил несколько недочетов и возможную ошибку :)
class Quote < ActiveRecord::Base
def pretty_turnaround
return "" if turnaround.nil?
offset = purchased_at || Time.now
#looks like errorr: (Time.now — purchased_at.to_time) / 60 / 60 / 24) is very small number
business_days = purchased_at? (Time.now — purchased_at.to_time) / 60 / 60 / 24).floor: turnaround
business_days += 1
time = offset + turnaround.days
time += 2.days if time.saturday?
time += 1.day if time.sunday?
#FIXME: may be use distance_in_words helper
"#{I18n.t(time)} (#{business_days} business days from today)"
end
end
выявил несколько недочетов и возможную ошибку :)
class Quote < ActiveRecord::Base
def pretty_turnaround
return "" if turnaround.nil?
offset = purchased_at || Time.now
#looks like errorr: (Time.now — purchased_at.to_time) / 60 / 60 / 24) is very small number
business_days = purchased_at? (Time.now — purchased_at.to_time) / 60 / 60 / 24).floor: turnaround
business_days += 1
time = offset + turnaround.days
time += 2.days if time.saturday?
time += 1.day if time.sunday?
#FIXME: may be use distance_in_words helper
"#{I18n.t(time)} (#{business_days} business days from today)"
end
end
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Секрет объектно-ориентированной разработки в Rails