views:

156

answers:

3

Hi,

I do not have problem as such but I am quite new to Ruby. I have the following 3 repeatable bits of code in one method and I would like to know how a true Rubyist would first of all remove the duplication and secondly make it more reusable.

Here is the code in question:

file = File.new( destination)
doc = REXML::Document.new file

doc.elements.each("configuration/continuity2/plans") do |element| 
  element.attributes["storebasedir"]  =  "#{TEST_OUTPUT_DIRECTORY}"
end

doc.elements.each("configuration/add").each do |database|
  database.raw_attributes = database.attributes.merge("connectionstring" => "#{TEST_CONNECTION_STRING}")
end

doc.elements.each("configuration/connectionStrings/plans") do |connectionString|
  connectionString.raw_attributes = connectionString.attributes.merge("connectionString" => "#{TEST_CONNECTION_STRING}")
end

Any advice appreciated.

Cheers

Paul

+2  A: 

You could try to add a method that tries to be as generic as possible to avoid this, but they are significantly different for me... You risk having complex code just to be able to wrap these lines into a single method.

Keltia
+2  A: 

I don't see the duplication. The collections you are iterating over and substantially different and the operation you are performing on each element is also very different. I agree with Olivier that any attempt at removing the duplication will simply result in more complex code.

Chris Johnston
+3  A: 

The last two blocks could be replaced with

["add", "connectionStrings/plans"].each do |elt_name|
  doc.elements.each("configuration/#{elt_name}").do |elt|
    elt.raw_attributes = elt.attributes.merge("connectionString" => "#{TEST_CONNECTION_STRING}")
  end
end

I assume the case difference between "connectionstring" and "connectionString" was accidental. If so, that exactly illustrates the benefit of removing duplication.

Also, you can probably replace "#{TEST_CONNECTION_STRING}" with TEST_CONNECTION_STRING.

Bkkbrad