Linking issues, classifying the relationship as a cause, effect, superset, etc

I first saw this gigantic if and tried to refactor it. Could only end with a endless switch statement.

Old code -

# It is a Cause
    if @causality == "C"  
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id
      @notice = 'New cause linked Successfully'
    end

    # It is an Inhibitor
    if @causality == "I"  
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id
      @relationship.relationship_type = 'I'
      @notice = 'New reducing issue linked Successfully'
    end        

    # It is a Superset
    if @causality == "P"  
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id
      @relationship.relationship_type = 'H'
      @notice = 'New superset linked Successfully'
    end

    # It is an Effect
    if @causality == "E"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid      
      @notice = 'New effect linked Successfully'
    end

    # It is an Inhibited
    if @causality == "R"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid      
      @relationship.relationship_type = 'I'
      @notice = 'New reduced issue linked Successfully'
    end

    # It is a Subset
    if @causality == "S"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid
      @relationship.relationship_type = 'H'          
      @notice = 'New subset linked Successfully'
    end

That's in one side of the if, in the 'else' side a similar piece of code goes like this

```
# Populate User_Id if relationship was created by a logged in User
if @issue.user_id.to_s != ""
@relationship.user_id = @issue.user_id
end

# It is a Cause
if @causality == "C"
@relationship.cause_id = @issue.id
@relationship.issue_id = @causality_id
@notice = 'New Issue was created and linked as a cause'
end

# It is an Inhibitor

I would actually take a more object oriented approach to this. Assume that your original object (the one you use to access the @relationship and @causality attributes) is named OriginalObject. In reality this could be a controller or simple Ruby object.

class OriginalObject
  attr_accessor :issue, :causality_id, :issueid

  def set_type_of_relationship(already_exists)
    obj = causality_object(already_exists)

    if obj.nil?
      @notice = 'Error creating and linking issue'
      return
    end

    @relationship.cause_id = obj.cause_id(self)
    @relationship.issue_id = obj.issue_id(self)
    @relationship.relationship_type = obj.relationship_type
    @notice = obj.notice
  end

  private

  def causality_object(already_exists)
    case @causality
    when 'C' then Cause
    when 'I' then Inhibitor
    when 'E' then Effect
    ...
    else nil
    end.new(already_exists)
  end
end

I would then subclass each cause object to remove the conditionals and simplify the code:

class CauseObject
  def initialize(already_exists)
    @already_exists = already_exists
  end

  def notice(created_and_linked, linked)
    if @already_exists
      "New Issue was created and linked as #{created_and_linked}"
    else
      "New #{linked} linked Successfully"
    end
  end
end

class CIPCauseObject < CauseObject
  def cause_id(obj)
    @already_exists ? obj.issueid : obj.issue.id
  end

  def issue_id(obj)
    obj.causality_id
  end
end

class ERSCauseObject < CauseObject
  def cause_id(obj)
    obj.causality_id
  end

  def issue_id(obj)
    @already_exists ? obj.issueid : obj.issue.id
  end
end

This would make it easy to create new Cause, Effect, Inhibitor, etc. objects.

class Cause < CIPCauseObject
  def relationship_type; nil; end

  def notice
    super('a cause', 'cause')
  end
end

class Inhibitor < CIPCauseObject
  def relationship_type; 'I'; end

  def notice
    super('a reducer', 'reducer')
  end
end

class Effect < ERSCauseObject
  def relationship_type; nil; end

  def notice
    super('an effect', 'effect')
  end
end

This approach allows for further functionality to be built on the cause objects without having to re-write that original conditional. For example the notice response could be modified without having to fit the original message formatting. It's also easier to unit test this code in isolation. In my opinion the set_type_of_relationship function is also much easier to read since there is no longer a large and confusing conditional in it.

StackExchange Code Review Q#6537, answer score: 2