An abundance of ternary operators

Question

I'd like to find a way to do what this method does in a cleaner, less hacky looking way

 def self.create_from_person!(person)
    spi = new(:person => person, :provider => person.provider)
    spi.age_eligible_code = participant.age_eligible?(person) ? 1 : 2
    spi.county_of_residence_code = participant.psu_county_eligible?(person) ? 1 : 2
    spi.first_prenatal_visit_code = participant.first_visit?(person) ? 1 : 2
    spi.pregnancy_eligible_code = participant.pbs_pregnant?(person) ? 1 : 2
    spi.save!
  end

Any thoughts?

Answer 1

Notes:

• Any reason not to set the attributes in a single create step?
• DRY by using procs for short-lived functions.
• Methods should return meaningful values. Here it makes sense to return the newly created object.
• (As Mark pointed out) The mismatch names between attributes makes the code more verbose that it should be. However, I guess part of this mismatch already comes from the app you are patching.
• :provider => person.provider. Note that this can break referential integrity. Person may change its provider in the future, but this model will remain as it is.

I'd write:

def self.create_from_person!(person)
  code = lambda { |value| value ? 1 : 2 }
  create!({
    :person => person, 
    :provider => person.provider,
    :age_eligible_code => code[participant.age_eligible?(person)],
    :county_of_residence_code => code[participant.psu_county_eligible?(person)],
    :first_prenatal_visit_code => code[participant.first_visit?(person)],
    :pregnancy_eligible_code => code[participant.pbs_pregnant?(person)],
  })
end

Answer 2

The reason why it doesn't look clean is because the underlying models are inconsistent.

In one, you return a boolean true/false for age_eligible? and in another, you call it age_eligible_code and store a 1 for true and 2 for false. If these were aligned such that they were both boolean values, you wouldn't need any ternary operators at all.