model methods - is this the best way?


I am putting quite alot of methods in my models such as User, for things such as:

class User < ActiveRecord::Base   .....

  # Count all new messagebox messages   def count_new_mbox     count_new_privmsgs + count_new_comments + count_new_requests   end

  # Count new private messages   def count_new_privmsgs     privmsgs.find(:all, :conditions => "is_unread = 1").size   end

  # Count number of new comments for the user   def count_new_comments     user_comments.find(:all, :conditions => "is_unread = 1").size   end end

is this the best way to go about implementing these methods? cos it seems to me that im going to have HEAPS of these methods in my model, for even my finds. eg. find_recent_comments(n), find_recent_privmsgs(n).

ahh wow, thanks didnt know about that. so will that return an array of Message objects ?

Consider good oo programming concepts while coding. The methods
related to messages should go into message model.

  It is unfortunate even plugins like restful authentication violate
basics of oo programming. It becomes even more difficult for newbies
to write good code.

To me it makes more sense to put all the methods relating a User to fetch and retrieve data inside the User.

Why would it be good oo programming to put these methods inside my message object?

So you mean to put inside message something like :

class Message < ActiveRecord::Base   def count_unread(user)       user.messages.find(:all, :conditions => "is_unread == true").size;   end end

I'm a bit new to Rails, but doesn't scoping let you do something like


(Assuming user has_many messages and message belongs_to user.) Then implement a class method similar to your method above.

class Message < ActiveRecord:Base   belongs_to => :user   def self.count_unread     find(:all, :conditions => {:is_unread => true}).count   end end

AFAIK, all finds on user.messages are scoped as if you had a condition :user_id =>, but I haven't looked at the details. Maybe someone else can confirm or correct me.

- Martin

Correct, also, the use of .size is a very bad solution, as it will effectively load all the records into memory (as an array of records). The count method will perform an SQL count.

Best regards

Peter De Berdt

ok thanks so much guys for your help, ill defiantly go about implementing my methods like that from now on,