current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I have n-times similar statements

    if trigger_data.tt_closed
      unless trouble_ticket.changes.key?(:status)
        @run = 0
        break
      end
      unless trouble_ticket.changes[:status][1] == "Closed"
        @run = 0
        break
      end
    end

    if trigger_data.tt_assignee
      unless trouble_ticket.changes.key?(:assigned_to)
        @run = 0
        break
      end
      unless trouble_ticket.changes[:assigned_to][1] == trigger_data.tt_assignee
        @run
        break
      end
    end

How to refactoring that code? Maybe dynamic statement build with pass some hash to input. I'm newbie in metaprogramming. Give me advise please

share|improve this question
    
Did you forget @run = 0 in the final one? –  rahul May 26 '12 at 22:49
add comment

1 Answer

up vote 1 down vote accepted

Are you inside a loop? Here is a possible way of doing it

def check(td, tt, trigger, key, ticket_changes)
    if td.send(trigger)
      unless tt.changes.key?(key)
        @run = 0
        return true
      end
      unless tt.changes[key][1] == ticket_changes
        @run = 0
        return true
      end
    end
    return false
end

def metacheck(td, tt)
    [[:tt_closed, :status, "Closed"],
     [:tt_assignee, :assigned_to, td.tt_assignee]].each do |k|
        return if check(td, tt, k[0], k[1], k[2])
     end
end


metacheck(trigger_data, trouble_ticket)

I have used an array of triplets to check the conditions. The check can further be simplified as 

def check(td, tt, trigger, key, ticket_changes)
    return false unless td.send(trigger)
    if !tt.changes.key?(key) or (tt.changes[key][1] != ticket_changes)
        @run = 0
        return true
    end
    return false
end

We can join all the conditions together. But I think this captures the original intension best.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.