Should I use a class method or an instance method and why?

Question

In my Rails application, when creating a business, I have a form that has the following field:

<%= check_box_tag(:default_company) %> <%= label_tag(:default_company, "Set Company as Default") %> 

Essentially, when I create a business, if they check this box, I need it to run something like the following code:

 def set_default_company(company, user) exists = DefaultCompany.find(user.id) if exists exists.update_attributes(company: company) else DefaultCompany.create(company: company, user: user) end end 

During training, I usually did this in my controller, but I try to follow best practices and use a thick model, a skinny controller, so I want to use this logic:

 def create @company = Company.new(params[:company]) if @company.save if params[:default_company] Company.set_default_company(@company.id, current_user.id,) end flash[:notice] = "Company was successfully created." redirect_to @company else redirect_to new_company_path end end 

That's where I got confused about whether to use a class method or an instance method to call set_default_company . They both seem to work, and I see no benefit to either.

In addition to giving me any information about which method to use, if someone can show me a brief implementation of writing this method as a class or instance method, it can give me a better idea of ​​why.

Here is how I wrote them:

 def self.set_default_company(company, user) # Logic here end def set_default_company(company, user) # Logic here end 

Having written them in this way, I also see no advantage.

Answer 1

As their name implies, the instance methods on the model should be used for logic / operations that apply to a particular user instance (the one on which the method is called.) Thus, you might think about setting up the default company for the user as the instance method on User Class methods are for things that don't work in a single instance of the model or in cases where you don't have an instance available to you. for example, you may have a class method for organizing your database, for example User.purge_expired_users , which will not be applied to a single user object.

eg.

 class User def set_default_company(company) exists = DefaultCompany.find(self.id) if exists exists.update_attributes(company: company) else DefaultCompany.create(company: company, user: self) end end end 

then your controller method will look like this:

 def create @company = Company.new(params[:company]) if @company.save if params[:default_company] current_user.set_default_company @company end flash[:notice] = "Company was successfully created." redirect_to @company else redirect_to new_company_path end end 

Alternatively, you could think of a relationship from a different perspective and put the instance method on Company for example. company.set_as_default_for(user) .

Answer 2

I would make the set_default_company instance method on User . A User defaults to Company ; Why is it necessary for Company , for which users is it by default?

 class User def set_default_company(company) exists = DefaultCompany.find(id) if exists exists.update_attributes(company: company) else DefaultCompany.create(company: company, user: self) end end end 

Answer 3

In my opinion, I always create a class method if the method in question represents information / behavior that is fairly common among all the instances created by the instance other than the instance methods that I use when I believe that this is more like the specific action of this object-object.

But this is my point of view.

Answer 4

A few things: do you have a separate table for DefaultCompany? It seems to be a logical flag in the company table.

Then is there a relationship between companies and users? If so, it seems the best way to do this would be

In user model

 def set_default_company(company) self.companies.each do |c| c.update_attributes(:default => false) end company.update_attributes(:default => true) end 

Or in a company model

 def set_as_default update_attributes(:default_company => true) end