Recently, I’ve vowed to remove most of the model.find methods from my controller. I am not talking about simple find statements like:
model.find(1)
model.find_by_title("my title")
I am talking about find methods like so:
model.find(:all, :conditions => ["status = ?", my_status]) model.find(:all, :conditions => ["title = ?", my_title])
I would add methods on the model called, find_all_by_status(status) and find_all_by_title(title). Now, you may be thinking that these are simple find statements too. And I would agree with you. But in my opinion it makes the controller code look dirty. What would you rather see in your controller?
model.find(:all, :conditions => ["status = ?", my_status]) ~OR~ model.find_all_by_status(my_status)
Which one can you comprehend faster? For me, the first statement takes some time for my brain to process, where as the second statement doesn’t not. Ok, if this isn’t enough to convince you. What if you introduce a new boolean attribute on your model called ‘active’ and the logic is to only show active model instances? Now you have to change all the places where you have:
model.find(:all, :conditions => ["status = ?", my_status]) ~TO~ model.find(:all, :conditions => ["status = ? and active = ?", my_status, true])
You say you were thinking ahead and have a private method on your controller to handle this call, so you only have to make the change in one place. I would say congratulations, but I would point out that I think the controller knows too much about the model and thus breaks encapsulation.
I would like to hear what other think, and how they handle this situation.
