I wrote a Rails model instance method that checks many separate conditions, and returns a single boolean. It does not employ any nesting of branching structures. Each check has its own comments describing its reasoning. Every check is necessary for the method to return an accurate value, and none of the checks are likely to be reused independently of the other checks. The function is 34 lines long including comments. A colleague reviewing the code suggested that the method may be too long, and therefore would be more readable if broken out into multiple helper methods for greater readability.def active?  #Check columns on self first, so if those dont pass,  #no other records will need to be loaded.  return false if self[:active] == false && !ignore_column  return false if self.suspended # here for thoroughness. tasks should never be marked active and suspended.  return false if self.completed_at # here for thoroughness. tasks should never be marked active and completed.  if failed_attempts >= MAX_FAILED_ATTEMPTS # here for thoroughness. tasks should never be marked active after the max failed attempts.    if !suppress_error_emails      SystemMailer.error message:Too many failed attempts for task execution. #{failed_attempts}/#{MAX_FAILED_ATTEMPTS} failed attempts for Task #{id}.    end    return false  end  #If the origin is inactive and not because it is a withdrawn status, then the task should be inactive.  return false if !evergreen && origin && !origin.active && origin.is_a?(Crm::Status) && !origin.indicates_closed # Status or Subscription  return false if sequenceable.is_a?(Crm::Case) && sequenceable.sales_stage_id.to_i>Crm::SalesStage.id('Placed') #placed policies shouldn't fire tasks  #Recruits are always marked inactive because they are users that can't log in.  #If the person is inactive but not a recruit, the task should be inactive.  is_a_recruit=person.try(:is_a?,Usage::User) && person.try(:role)==Usage::Role['recruit']  return false if !person.try(:active) && !is_a_recruit  if self.template    template_purpose_id=self.template.template_purpose_id    purposes=Marketing::Template::TEMPLATE_PURPOSE    status_opted_out=self.person.status_unsubscribe && template_purpose_id==purposes[Status][:id]    con_mktg_opted_out=self.person.is_a?(Crm::Connection) && self.person.marketing_unsubscribe && template_purpose_id==purposes[Consumer Marketing][:id]    user_mktg_opted_out=self.person.is_a?(Usage::User) && self.person.marketing_unsubscribe && template_purpose_id==purposes[Agent Marketing][:id]    if status_opted_out || con_mktg_opted_out || user_mktg_opted_out      return false    end  end  return trueendIs there a magic number of lines where a function or method becomes too long and should be broken out into multiple functions? If so, what is the magic number? Does it depend on the type of code (model/view/controller), or some other circumstance?
Determining whether a task is active based on many conditions
ruby;ruby on rails