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.

I have a ruby code segment which I need to DRY up. Following is the code segment.

def disable_attribute
  statement1
  @response = Client::Service.disable_attribute(
    param1: param1value,
    param2: param2value
  )   
  statement2
end

def enable_attribute
  statement1
  @response = Client::Service.enable_attribute(
    param1: param1value,
    param2: param2value
  )   
  statement2
end
share|improve this question
    
those methods have no arguments for the attribute that's being updated? –  tokland Feb 28 at 9:26
    
"Without metaprogramming"? This is per definition meta as it is about the method name, which is outside the problem domain. –  demonkoryu Feb 28 at 13:03
add comment

2 Answers

I'd write:

def set_attribute(value)
  statement1
  method = value ? :enable_attribute : :disable_attribute
  @response = Client::Service.send(method, param1: param1value, param2: param2value)
  statement2
end

def disable_attribute
  set_attribute(false)
end

def enable_attribute
  set_attribute(true)
end
share|improve this answer
    
Why did you choose to pass true/false rather than the applicable method? –  Cary Swoveland Mar 3 at 5:38
    
@CarySwoveland: This way that info (the name of the methods) is contained in a single place and those other two methods don't need to know any details. –  tokland Mar 3 at 9:02
add comment

How about:

def toggle_attribute
  statement1
  @response = yield(
    param1: param1value,
    param2: param2value
  )
  statement2
end

def disable_attribute
  toggle_attribute { |params| Client::Service.disable_attribute(params) }
end

def enable_attribute
  toggle_attribute { |params| Client::Service.enable_attribute(params) }
end

If you want to be more succinct, you could:

def toggle_attribute(method_name)
  statement1
  @response = Client::Service.send(method_name,
    param1: param1value,
    param2: param2value
  )
  statement2
end

def disable_attribute
  toggle_attribute(:disable_attribute)
end

def enable_attribute
  toggle_attribute(:enable_attribute)
end

And if you want to really impress your friends, you could write:

def toggle_attribute(method_name)
  statement1
  @response = Client::Service.send(method_name,
    param1: param1value,
    param2: param2value
  )
  statement2
end

def method_missing(method_name, *args)
  %i[disable_attribute enable_attribute].include?(method_name) ? 
    toggle_attribute(method_name) : super
end
share|improve this answer
1  
Nice answer, Uri. I'd definitely vote for the second option. –  Jonah Mar 1 at 19:11
    
@Jonah: method_missing is a basic example of metaprogramming ;) –  tokland Mar 3 at 9:05
    
That was the 3rd option, I voted 2nd –  Jonah Mar 3 at 18:30
    
Great answer! I don't think I'd use the term toggle in the method name, though, because the method doesn't toggle. Perhaps set or update. –  Mark Thomas Mar 19 at 1:05
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.