Best way to insert some functionality into an existing ruby method

Question

I am working with a legacy method that filters out some data and returns an array of elements.

Simplified version looks like:

def filtering_rules
  items = []
  items = apply_rule_one
  return items if items == 3

  items = apply_rule_two
  return items if items == 3

  items = apply_rule_three  
  return items if items == 3
end

I need to add a logger before apply_rule three is run so I did this:

 def filtering_rules
  items = []
  items = apply_rule_one
  return items if items == 3

  items = apply_rule_two
  return items if items == 3

  Rails.logger("Rule 1 and 2 failed to return data moving to rule 3") if items.empty?  
  items = apply_rule_three  
  return items if items == 3
end

My tests are passing and things are working. But the code is not DRY and the logger inside the rules filter is ugly. Any suggestions as far as patterns or best practices are concerned?

What's not dry about this? Where have you repeated yourself? — boulder
– boulder, Commented Apr 19, 2013 at 13:01

boulder · Accepted Answer · 2013-04-19 13:12:05Z

1

How about this?

RULES = [:rule_one, :rule_two, :rule_three]

def filtering_rules
 RULES.each do |rule|
   items = self.send("apply_#{rule}".to_sym)
   return items if items == 3
 end
end

And put your logger into the last rule. Now, is this worth it? My guess is that only if you expect rules can grow overtime.

answered Apr 19, 2013 at 13:12

boulder

3,27617 silver badges21 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

RidingRails Over a year ago

Good Question... Well we started with no rules and we have 3 rules a few months after going live. I am guessing more are on the way. Thanks for for the help.

Senad Uka Over a year ago

btw. putting logger in last rule is not good since it contains information about other rules ? what if rule 3 is called in other context - why would it contain information failing to return data from other rules ...

Senad Uka · Accepted Answer · 2013-04-19 13:12:52Z

0

Well we don't know much about apply_rule_x functions and the broader context - but you could write this like:

items = [] # assuming it's an array originaly
[:apply_rule_one,:apply_rule_two, :apply_rule_three].each do |rule|
     Rails.logger("Rule 1 and 2 failed to return data moving to rule 3") if items.empty? and rule == :apply_rule_three
    items = send(rule)
    return items if items == 3
end

However if you only have 3 rules and number won't be increased - it's totally ok to leave the code as it is.

answered Apr 19, 2013 at 13:12

Senad Uka

1,1317 silver badges18 bronze badges

1 Comment

RidingRails Over a year ago

Yes exactly. Thanks for the help. I hard coded rule three for illustration purposes.

Collectives™ on Stack Overflow

Best way to insert some functionality into an existing ruby method

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related