Should I use class method or instance method, and

2020-05-19 08:37发布

问题:

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.

回答1:

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).



回答2:

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


回答3:

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.



回答4:

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