CODEWITHSUNDEEP
ruby-on-railsrubyoopmodel-view-controller
ruevaughn
ruevaughn

Reputation: 1319

Should I use class method or instance method, and why?

In my Rails app, 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

While learning, I would usually do that stuff in my controller but i'm trying to follow best practices and use a fat model, skinny controller, so I'm wanting to use logic like this:

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

Here is where I am getting confused on whether to use a class method or an instance method, to call set_default_company. They both seem like they would work and I can't see a benefit to one or the other.

In addition to giving me any information as to which method to use, if someone could show me a brief implementation of writing that as a class method vs. instance method it may give me a better understanding as to why.

Here is how I would write them:

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

def set_default_company(company, user)
  # Logic here
end

Writing them that way I don't see a benefit to either.

Upvotes: 9

Views: 14411

Answers (4)

MurifoX
MurifoX

Reputation: 15089

In my opinion, I always create a class method if the method in question represents information/behavior that is quite generic among all the objects instantiated, different from the instance methods, that I use when I believe it's more like a specific action of the instantiated object in question.

But that is my point-of-view.

Upvotes: 2

mikej
mikej

Reputation: 66263

As their name suggests, instance methods on a model should be used for logic/operations that relate to a specific instance of a user (the one on which the method is called.) So you could think of setting the default company for a user as an instance method on User. Class methods are for things which don't operate on an individual instance of a model or for cases where you don't have the instance available to you. e.g. you might have a class method to tidy up your database such as User.purge_expired_users which would not apply to an individual user object.

e.g.

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 would look like:

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 the relationship from the other perspective and put an instance method on Company e.g. company.set_as_default_for(user).

Upvotes: 20

Solomon
Solomon

Reputation: 7023

A few things: do you have a separate table for DefaultCompany? This seems like it should be a boolean flag on the company table.

Next, is there an association between companies and users? If so, it seems the best way to do it would be

In the 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 the Company model

def set_as_default
  update_attributes(:default_company => true)
end

Upvotes: 1

icecream
icecream

Reputation: 1425

I would actually make set_default_company an instance method on User. A User has a default Company; why should a Company need to what users it is default for?

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

Upvotes: 4

Related Questions