ruby - refactoring if else statement

Question

I've tried reading some tutorials on refactoring and I am struggling with conditionals. I don't want to use a ternary operator but maybe this should be extracted in a method? Or is there a smart way to use map?

detail.stated = if value[:stated].blank?
                  nil
                elsif value[:stated] == "Incomplete"
                  nil
                elsif value[:is_ratio] == "true"
                  value[:stated] == "true"
                else
                  apply_currency_increment_for_save(value[:stated])
                end

Answer 1

If you move this logic into a method, it can be made a lot cleaner thanks to early return (and keyword arguments):

def stated?(stated:, is_ratio: nil, **)
  return if stated.blank? || stated == "Incomplete"
  return stated == "true" if is_ratio == "true"
  apply_currency_increment_for_save(stated)
end

Then...

detail.stated = stated?(value)

Answer 2

stated = value[:stated]
detail.stated = case
  when stated.blank? || stated == "Incomplete"
    nil
  when value[:is_ratio] == "true"
    value[:stated] == "true"
  else
    apply_currency_increment_for_save stated
end

What's happening:   when case is used without an expression, it becomes the civilized equivalent of an if ... elsif ... else ... fi.

You can use its result, too, just like with if...end.

Answer 3

Move the code into apply_currency_increment_for_save and do:

def apply_currency_increment_for_save(value)
  return if value.nil? || value == "Incomplete"
  return "true" if value == "true"
  # rest of the code. Or move into another function if its too complex
end                         

The logic is encapsulated and it takes 2 lines only

Answer 4

I like @Jordan's suggestion. However, it seems the call is incomplete -- the 'is_ratio' parameter is also selected from value but not supplied.

Just for the sake of argument I'll suggest that you could go one step further and provide a class that is very narrowly focused on evaluating a "stated" value. This might seem extreme but it fits with the notion of single responsibility (the responsibility is evaluating "value" for stated -- while the 'detail' object might be focused on something else and merely makes use of the evaluation).

It'd look something like this:

class StatedEvaluator
  attr_reader :value, :is_ratio

  def initialize(value = {})
    @value = ActiveSupport::StringInquirer.new(value.fetch(:stated, ''))
    @is_ratio = ActiveSupport::StringInquirer.new(value.fetch(:is_ratio, ''))
  end

  def stated
    return nil if value.blank? || value.Incomplete?
    return value.true? if is_ratio.true?
    apply_currency_increment_for_save(value)
  end
end

detail.stated = StatedEvaluator.new(value).stated

Note that this makes use of Rails' StringInquirer class.