# users controller
def dashboard
@submitted_assignments = Assignment.find(:all, :joins => :student_assignments)
@courses = current_user.classrooms.find(:all, :include => :assignments)
end
# dashboard.html.haml
- for course in @courses
- for assignment in course.assignments
- for sa in assignment.student_assignments
= sa.id
Refactorings
No refactoring yet !
Dmitry Polushkin
February 25, 2009, February 25, 2009 05:43, permalink
Why do you think is it bad?
fnjord.myopenid.com
February 25, 2009, February 25, 2009 05:49, permalink
Just seems like too many steps to get to the records I am after. Maybe this isn't so bad after all?
Fabien Jakimowicz
February 25, 2009, February 25, 2009 14:20, permalink
You can optimize a little bit your code but you will have to modify your models.
Take a look at :through argument in ActiveRecord::Base/has_many documentation.
PS: if you precise a little bit your models and relations, maybe we can find a better solution
class User < ActiveRecord::Base has_many :classrooms has_many :assignments, :through => :classrooms end
class UsersController < ApplicationController
def dashboard
@submitted_assignments = Assignment.find(:all, :joins => :student_assignments)
@assignments = current_user.assignments
end
end
- for assignment in @assignments
- for sa in assignment.student_assignments
= sa.id
Adam
February 25, 2009, February 25, 2009 16:04, permalink
Fabien has the right idea. However, I feel that you may want to drill it down even further. I will leave the model as an exercise for you as I do not have enough information about your data structure to make any specific recommendations.
class StudentAssignmentsController < ApplicationController
def index
@student_assignments = current_user.student_assignments
end
end
- for student_assignment in @student_assignments = student_assignment.id
Blaxter
March 3, 2009, March 03, 2009 09:12, permalink
I agree with Adam's solution, you are doing to much in the view, keep it as simple as you can and do the work in the model.
Granito Silestone
January 30, 2010, January 30, 2010 01:02, permalink
I just came across this, Fabien's idea helped. Thanks!
I am trying to find a reasonable way to get to the target data in this code. It works, but I don't like how it looks are surely can be refactored. I have experimented, but have had little success. The markup in the views is haml, if you are unfamiliar. Looking forward to refactoring this!