将问题联系起来，将关系划分为因果关系、超集关系等。

Question

我第一次看到这个巨大的“如果”并试图重构它。只能以一个没完没了的开关语句结束。

旧代码-

  # 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 

在if的一侧，在“one”端，类似的代码如下所示

 # 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
      if @causality == "I"  
        @relationship.cause_id = @issue.id
        @relationship.issue_id = @causality_id
        @relationship.relationship_type = 'I'            
        @notice = 'New Issue was created and linked as reducer'
      end

      # It is a Superset
      if @causality == "P"  
        @relationship.cause_id = @issue.id
        @relationship.issue_id = @causality_id 
        @relationship.relationship_type = 'H'
        @notice = 'New Issue was created and linked as a superset'
      end         

      # It is an Effect
      if @causality == "E"  
        @relationship.cause_id = @causality_id
        @relationship.issue_id = @issue.id
        @notice = 'New Issue was created and linked as an effect'
      end          

      # It is an Inhibited
      if @causality == "R"  
        @relationship.cause_id = @causality_id
        @relationship.issue_id = @issue.id
        @relationship.relationship_type = 'I'
        @notice = 'New Issue was created and linked as reduced'
      end              

      # It is a Subset
      if @causality == "S"  
        @relationship.cause_id = @causality_id
        @relationship.issue_id = @issue.id
        @relationship.relationship_type = 'H'
        @notice = 'New Issue was created and linked as a subset'
      end  

新代码-

def set_type_of_relationship(already_exists)
  if !already_exists
    case @causality
    when "C"       
      @relationship.cause_id = @issue.id
      @relationship.issue_id = @causality_id  
      @notice = 'New Issue was created and linked as a cause'
    when "I"
      @relationship.cause_id = @issue.id
      @relationship.issue_id = @causality_id  
      @relationship.relationship_type = 'I'            
      @notice = 'New Issue was created and linked as reducer'
    when "P"
      @relationship.cause_id = @issue.id
      @relationship.issue_id = @causality_id  
      @relationship.relationship_type = 'H'
      @notice = 'New Issue was created and linked as a superset'
    when "E"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issue.id
      @notice = 'New Issue was created and linked as an effect'
    when "R"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issue.id
      @relationship.relationship_type = 'I'
      @notice = 'New Issue was created and linked as reduced'
    when "S"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issue.id
      @relationship.relationship_type = 'H'
      @notice = 'New Issue was created and linked as a subset'
    else 
      @notice = 'Error creating and linking issue'
    end
  else #if already_exists
    case @causality
    when "C"       
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id
      @notice = 'New cause linked Successfully'
    when "I"
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id
      @relationship.relationship_type = 'I'
      @notice = 'New reducing issue linked Successfully'
    when "P"
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id
      @relationship.relationship_type = 'H'
      @notice = 'New superset linked Successfully'
    when "E"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid      
      @notice = 'New effect linked Successfully'
    when "R"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid      
      @relationship.relationship_type = 'I'
      @notice = 'New reduced issue linked Successfully'
    when "S"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid
      @relationship.relationship_type = 'H'          
      @notice = 'New subset linked Successfully'
    else 
      @notice = 'Error creating and linking issue'
    end 
  end
end

这个没完没了的开关(case)语句让我有点疯狂，但我没有办法将庞大的ifs列表转换为更容易调试的东西，更重要的是，更少的重复。我选择将所有内容提取到两个开关中，主要是因为交换机是使用Ruby中的索引分支表实现的，也就是说，它的实现速度相当快。仔细看看这个，帮我重建它！

注意:注意两者之间的差异

@relationship.cause_id = @issueid
@relationship.issue_id = @causality_id

和

@relationship.cause_id = @causality_id
@relationship.issue_id = @issue.id

Answer 1

我实际上会采取一种更面向对象的方法来解决这个问题。假设您的原始对象(用于访问@relationship和@因果关系属性的对象)名为OriginalObject。实际上，这可能是一个控制器或简单的Ruby对象。

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

然后，我将对每个原因对象进行子类，以删除条件并简化代码：

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

这将使创建新的Cause、Effect、Inhibitor等对象变得容易。

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

这种方法允许在原因对象上构建更多的功能，而不必重写原始条件。例如，可以修改通知响应，而不必符合原始消息格式。隔离地对此代码进行单元测试也更容易。在我看来，set_type_of_relationship函数也更容易阅读，因为它中不再有一个大的和令人困惑的条件。

Answer 2

这里的这块：

if already_exists
  case @causality 
  when "C", "I", "P"
  @relationship.cause_id = @issueid
  @relationship.issue_id = @causality_id 
  when "E", "R", "S"
  @relationship.cause_id = @causality_id
  @relationship.issue_id = @issueid
  end   
  @relationship.relationship_type = args[0].try(:to_s)            
else
  case @causality 
  when "C", "I", "P"
  @relationship.cause_id = @issue.id
  @relationship.issue_id = @causality_id 
  when "E", "R", "S"
  @relationship.cause_id = @causality_id
  @relationship.issue_id = @issue.id
  end
  @relationship.relationship_type = args[0].try(:to_s)            
end

这是：

issue_id = already_exists ? @issueid : @issue.id
ids = [issue_id, @causality_id]
ids.rotate! if %W[E R S].member? @causality
@relationship.cause_id, @relationship.issue_id = ids
@relationship.relationship_type = args[0].try(:to_s)            

(未经测试，但相当接近。)

令人感兴趣的是，这是两者之间的中间步骤。

if already_exists
  vars = [@issueid, @causality_id]
  vars.rotate! if %W[E R S].member? @causality
  @relationship.cause_id, @relationship.issue_id = vars
else
  vars = [@issue.id, @causality_id]
  vars.rotate! if %W[E R S].member? @causality
  @relationship.cause_id, @relationship.issue_id = vars
end
# Don't know why you repeated this.
@relationship.relationship_type = args[0].try(:to_s)            

对我来说，这些事情是分阶段进行的。我注意到的第一件事是，这两个块的逻辑是相同的，只是使用不同的值。在我的脑海里：

• 每个块使用不同的值。
• 只有一个是不同的
• @causality会使它们翻转
• Ruby有一种简单的表达方式

Answer 3

好的，我是这个问题的作者，但我对CR没有任何意见，这是最后的重构:)

def set_type_of_relationship(already_exists)
    args = { 
        C: [nil, 'a cause',    'cause'],
        I: [:I,  'a reducer',  'reducer issue'],
        P: [:H,  'a superset', 'superset'],
        E: [nil, 'an effect',  'effect'],
        R: [:I,  'reduced',    'reduced issue'],
        S: [:H,  'a subset',   'subset'] 
      }[@causality.to_sym]

    (@notice = 'Error creating and linking issue' and return) if args.nil?

    if already_exists
      case @causality 
      when "C", "I", "P"
      @relationship.cause_id = @issueid
      @relationship.issue_id = @causality_id 
      when "E", "R", "S"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issueid
      end   
      @relationship.relationship_type = args[0].try(:to_s)            
    else
      case @causality 
      when "C", "I", "P"
      @relationship.cause_id = @issue.id
      @relationship.issue_id = @causality_id 
      when "E", "R", "S"
      @relationship.cause_id = @causality_id
      @relationship.issue_id = @issue.id
      end
      @relationship.relationship_type = args[0].try(:to_s)            
    end

    @notice = if already_exists
                "New Issue was created and linked as #{args[1]}"
              else
                "New #{args[2]} linked Successfully"
              end
  end