Topic: Mixing Logic in Models

I have this query in my view page that finds all the users for the site, orders them by last name and then checks to see if that user has "0" projects associated with them. If this is true then we spit out the names of those people.

#/layouts/projects.rhtml
<h1>Users who don't have projects assigned to them</h1>
<% for user in User.find(:all, :order=>'last_name') %>
    <% if user.projects.count == 0 %>
        <%= user.first_name %><br>
    <% end %>
<% end %>

I look at this code and I think this type of query is not DRY and should be in a model and have this type of method.

class User < ActiveRecord::Base
  def self.not_assigned_to_projects
     # My attempt would be to put the code within this model file
    # I just can't figure out how could I get this "user.projects.count == 0" inside this method/model?
  end
end

I just can't figure out how could I get this "user.projects.count == 0" inside this method/model? Can that logic be put into this model/method? OR Is it best practice to put this type of logic inside of a controller or view?

Looking to see how someone else might approach this?

Re: Mixing Logic in Models

LOL Rails is awesome I think I just figured out a solution to my own problem:)
I would like anyone elses opinion on it. It seems to work well

So my new problem is how can I find out what users don't have any projects associated with them. The new solution is as follows.

# MODEL
class User < ActiveRecord::Base
  def self.find_all_users
     find(:all, :order=>'last_name ASC')
   end

  def self.non_active_users
   # I just want to find out the users who DO NOT HAVE projects assigned to them
   find_all_users.reject { |users| users.projects.count !=0 }
   end
end

#CONTROLLER
# projects_controller.rb

  def list
    @users_with_no_projects = User.non_active_users
  end


#VIEW
# view/projects/list.rhtml
    <strong>Users with no projects</strong><br>
    <% for user in @users_with_no_projects %>
        <% user.first_name %><br>
    <% end %>


It works well, I just wonder if this solution is DRY or can someone see a better way to structure it.

Re: Mixing Logic in Models

Might I suggest using a counter cache? Basically you'd put a project_count field in your Users table and define the :counter_cache option in your has_many association. Then you can just search the Users table without loading Projects as well, which will reduce the overhead you have right now in loading user and project data, and you won't have to loop through each user before returning a list of good results.

vinnie - rails forum admin

Re: Mixing Logic in Models

Hey that sounds like an interesting solution. I never heard of a :counter_cache.

So if I was to implement this solution I would need to:
1) Alter my User table to have a column called "project_count"
2) Alter my User Model's HABTM to have the :counter_cache defined.

Without investigating this too much, would I need to put some logic somewhere else to tell the app that when a Project is created and the user(s) are added/removed to a project please update each users "project_count" by +1 if adding or -1 if removing ?

Or

Is "project_count" and the HABTM :counter_cache, have this magic convention that takes care of all of this for me?

I hope I'm making sense. Still learning but this sounds awesome.

Re: Mixing Logic in Models

A counter cache sounds good. An alternative is this query:

find(:all, :include => :projects, :conditions => 'projects.id IS NULL', :order=>'last_name ASC')

Untested

Railscasts - Free Ruby on Rails Screencasts

Re: Mixing Logic in Models

Hey Ryan B and Vin. Thanks for the posts. Here is another question in re: to maybe a DRY principle or maybe even just a DB optimization issue.

So I have a "USER" list view. It pulls in all the users in along with projects associated with them. So at this point, I want to filter this result into results. One to show a list of people who are assigned to projects another to show people who are NOT assigned to projects. From the code below, what makes sense with keeping things DRY.

CONCEPT 1

class User < ActiveRecord::Base
  def self.find_all_users
      find(:all, :order=>'last_name ASC')
    end
 
def self.non_active_users
# I just want to find out the users who DO NOT HAVE projects assigned to them
    find_all_users.reject { |users| users.projects.count !=0 }
   end
end

# #CONTROLLER
# # projects_controller.rb
 
   def list
     @users_with_no_projects = User.non_active_users
   end
 
 
# #VIEW
# # view/projects/list.rhtml
  <strong>Users with no projects</strong><br>
     <% for user in @users_with_no_projects %>
     <% user.first_name %><br>
   <% end %>


CONCEPT 2
class User < ActiveRecord::Base
 
def self.find_all
    find_all(:all)
   end
end

# #CONTROLLER
# # projects_controller.rb
   def list
     @projects = User.find_all
   end
 
 
# #VIEW
# # view/projects/list.rhtml
  <strong>Users with no projects</strong><br>
     <% for user in  @projects.reject { |users| users.projects.count !=0 }%>
     <% user.first_name %><br>
   <% end %>

  <strong>Users with projects</strong><br>
     <% for user in  @projects %>
     <% user.first_name %><br>
   <% end %>


Overall, I'm trying to figure out, can I hit the DB ONLY ONCE, get the entire list and then parse the list to show what I need to show in my view(s).

My original solution I came up with seems semantically okay but I'm hitting the DB 4, 5 times asking for pretty much the same list just parsed differently? (i.e. Users with projects, Users with no projects)

I hope I'm making sense with this concept? Does anyone have a best practice approach to this at all?

Re: Mixing Logic in Models

If you are going to display all of the users and load them all into memory anyway, hitting the database once and parsing the results is probably faster. But, I wouldn't worry about it too much until you know performance will be a problem.

One thing to consider is if the table gets large and you want to add pagination or something you will have to move back to a database query so you can limit the number of results the same time you specify the conditions.

Edit: BTW, calling user.projects.count will hit the database every time it is called (for every user) unless you have a counter cache or include the projects when fetching the users.

Last edited by ryanb (2007-01-03 18:32:47)

Railscasts - Free Ruby on Rails Screencasts