Refactoring a if statement having multiple 'and' and 'or' statements [closed]

Question

Closed. This question does not meet Stack Overflow guidelines. It is not currently accepting answers.

---

This question does not appear to be about programming within the scope defined in the help center.

Closed 9 years ago.

Improve this question

What is best way to refactor or make more readable this chunk of code in ruby ?

 def check(message_type)
    if (message_type == 'reminder' and self.copy_reminder == true) or
            (message_type == 'rrm' and self.is_rrm == true and self.copy_rrm == true) or
            (message_type == 'alert' and self.is_rrm == true and self.copy_alert == true) or
            (message_type == 'reply' and self.is_rrm == true and self.copy_user_response == true)
          call_some_method
end

Answer 1

Do not use and or or in boolean condition, use && and ||. and and or are for control flow. Do not use self. when reading an attribute, you just do not need it. Do not check booleans like this some_thing == true, some_thing is trueish enough.

And I prefer to move complex conditions into private methods. That keeps the method that actual does something more readable.

  def check(message_type)
    call_some_method if valid_message_type?(message_type)
  end

private

  def valid_message_type?(message_type)
    case message_type
    when 'reminder' then copy_reminder
    when 'rrm'      then is_rrm && copy_rrm
    when 'alert'    then is_rrm && copy_alert
    when 'reply'    then is_rrm && copy_user_response
    end
  end