Creating a percentage complete method for a form I am a relatively new Rails developer and doctor doing research. In my Rails based research projects I often have questionnaires for which I provide a "% complete" parameter.

Here is an example.  You can see that each of the score attributes is given a value and then I can create a %. It looks awful and very un-Ruby-like. I would be keen once and for all to get an experts view and give me some steerage on how this could be cleaned up.

This method is in the Patient class:

 def percent_complete(user, patient)
   score = Score.where(user_id: user.id, patient_id: self.id).first
  if score.nil?
    return "0"
  end
  @status = 0
   score_array = [score.dx1, score.dxcon1, score.db1, score.dbcon1, score.biopsy, score.mgt]
   score_array.each do | element|
    if element == score.dx1
      @status = @status + 20
    elsif element == score.db1
      @status = @status + 20
    elsif element == score.dxcon1
      @status = @status + 20
    elsif element == score.dbcon1
      @status = @status + 20
    elsif element == score.biopsy
      @status = @status + 10
    elsif element == score.mgt
      @status = @status + 10
    end
   end
  @status
 end


Each Score belongs to a Patient (the Patient has many scores). Each Score belongs to a User (a User has many scores).


  score.dx1, score.dxcon1, score.db1, score.dbcon1, score.biopsy, score.mgt


These are the Score class attributes:


dx1 = diagnosis1 (string)
dxcon1 = diagnostic confidence1 (integer)
db1 = disease behaviour1 (string)
dbcon1 = confidence on disease behaviour (integer)
biopsy = should you biopsy? (boolean)
mgt = management (string)


I am now trying something like this:

 def percent_complete(user, patient)
    score = Score.where(user_id: user.id, patient_id: self.id).first
    if score.nil?
      return "0"
    end
    score_completeness = [score.dx1, score.dxcon1, score.db1, score.dbcon1, score.biopsy, score.mgt]
    @status = 0
    score_completeness.each do | field |
       @status += (100/6.0) unless field.blank?  
    end
   @status
 end
Looking only at your last block:


Why return the string "0" instead of the number?
The hard coded array is cryptic, and a potential maintenance hassle. I'd recommend moving it to a method on the Score model to abstract things.
The hardcoded 100/6.0 is basically a magic number. It'd be better to get the length of whatever array you're working with than assume it's always 6.
Why is @status an instance variable? Your method is meant to return something. If it returns at the first return line, you never even set @status to anything, which'll lead to errors later on, if you expect it to be set. Conversely, if you don't expect it to be set, or don't intend to use it, why set it?


I imagine you could do something like this:

def percent_complete(user, patient)
  score = Score.where(user_id: user.id, patient_id: self.id).first
  return 0 if score.nil? || score.steps.empty?
  100.0 * score.completed_steps.length / score.steps.length
end


where Score#steps and Score#completed_steps look like:

def steps
  [dx1, dxcon1, db1, dbcon1, biopsy, mgt]
end

def completed_steps
  steps.reject(&amp;:blank)
end