I am a beginner on rails. I work on a little application and i have a design problem. I think i have too much logic in my view and it should not be done like that.
Here my controller :
if params[:search] @hosts=Host.all @total = {} @total_by_group={} @search=true Disks.search(params[:search]).each do |disk| if @total[disk.host.name] @total[disk.host.name]+=disk.capacit开发者_如何转开发y else @total[disk.host.name]=disk.capacity end if @total_by_group[disk.group.name] @total_by_group[disk.group.name]+=disk.capacity else @total_by_group[disk.group.name]=disk.capacity end end end
And my view :
- if @search - @hosts.each do |host| - if @total[host.name] %br %table %tr %th host %th total size - host.groups.each do |group| - if @total_by_group[group.name] %th=group.name %tr %td=host.name %td=sprintf("%0.02f", @total[host.name]) - host.groups.each do |group| - if @total_by_group[group.name] %td=sprintf("%0.02f", @total_by_group[group.name])
It's working correctly but doesn't feel right. I think my view need to be more simpler.
I searched and i found some solutions where people build a model to store the results but it seems to be overkill to me and add a lot of works on schema when i change my requests and need to have some way to clean up the table after some time.
What is the rails way to do something like that ?
Thanks
Alain
Really the only thing you should have in your controller is this:
if params[:search]
@disks = Disks.custom_search(params[:search])
end
and in your disks model something like
def custom_search term
find_by_field(term, :include => :host)
Everything else should be in helper methods to which you pass the disks variable and are returned the calculated result for the view. The @search instance variable isn't necessary as params are available in the view and helper methods.
Ok from what you tell me this first example should be in the model. Remember that any representation of model data should come from your models, even calculations of data which aren't stored in your database.
Something like:
#view
- @disks.collect(&:host).each do |host|
- host.groups.each do |group|
= group.name
= group.disks_capacity
#your model group.rb
def disks_capacity
disks.map{|disk| disk.capacity}.sum # with disks.map we're talking about the disks which belong to this instance of group
end
A helper method is defined by adding a method to a xyz_helper.rb.
If I can make an honest suggestion that you buy a book and work through it. I can guarantee you'll enjoy the experience of learning rails, make things a lot easier for yourself and save a lot of time in the long run.
I tried to improve a little my code based on mark's comments.
I removed some code from controller :
if params[:search] @disks= Disks.custom_search(params[:search]) end
I added some helpers :
def host_total(host) @disks.host(host).sum(:capacity) end def list_hosts @disks.joins(:hosts).group("hosts.name").select("hosts.name") end def list_dgroup_by_host(host) @disks.host(host).group("dgroups.name").select("dgroups.name") end def capacity_by_dgroup(group) @disks.by_dgroup(group).sum(:capacity) end
And here my partial view :
- list_hosts.each do |host| %br %table %tr %th host %th total size - list_dgroup_by_host(host.name).each do |group| %th=group.name %tr %td=host.name %td= host_total(host.name)) - list_dgroup_by_host(host.name).each do |group| %td=capacity_by_dgroup(group.name)
I think it can be improved but i don't see how. If someone can suggest something, i will be glad :-)
精彩评论