Связывая вопросы, классифицирующие отношения как причина, следствие, суперсет и т. д.


Я впервые увидел эту гигантскую, если и пытались переделать его. Может закончиться только с бесконечным оператора switch.

Старый код -

  # 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 

Это в одну сторону если в 'другой' стороны подобный кусок кода выглядит так

 # 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

Этот бесконечный переключатель (дела) заявление сводило меня с ума немного, но я не нашел способ перевести огромный список МФС ничего проще для отладки и, что еще важнее, с меньшим дублированием. Я решил извлечь все на два переключателя главным образом потому, что переключатели выполнены с индексированной таблице в рубиново – я.е ее довольно быстрее. Внимательно посмотреть на это и помочь мне переделать его!

Внимание: обратите внимание на различия между

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

и

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


Комментарии
3 ответа

Я бы на самом деле сделать более объектно-ориентированного подхода к этому. Предположим, что ваш исходный объект (тот, который вы используете для доступа к @отношениях и @причинности атрибутов) называется 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

Это позволит легко создавать новые причины, эффект, ингибитори т. д. объекты.

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 функция тоже значительно облегчает чтение, поскольку нет более большой и непонятный условный в нем.

2
ответ дан 5 декабря 2011 в 07:12 Источник Поделиться

Этот фрагмент Здесь:

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)

Для меня эти вещи поэтапно. Первое, что я заметил, что логика два куска была одинакова, просто используя разные значения. В моей голове:


  • Каждый блок использует разные значения

  • Только одно отличается от другого.

  • @причинности заставляет их перевернуть

  • Ruby имеет простой способ, чтобы выразить это

1
ответ дан 5 декабря 2011 в 05:12 Источник Поделиться

Ок, я автор вопроса, но у меня нет аккаунта на СГ, это последний рефакторинг :)

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

0
ответ дан 5 декабря 2011 в 03:12 Источник Поделиться