CODEWITHSUNDEEP
rubyoptimization
dagda1
dagda1

Reputation: 28800

How can I refactor this piece of Ruby code to remove duplication?

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.

Upvotes: 1

Views: 259

Answers (3)

Bkkbrad
Bkkbrad

Reputation: 3147

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.

Upvotes: 3

Chris Johnston
Chris Johnston

Reputation: 1919

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.

Upvotes: 2

Keltia
Keltia

Reputation: 14743

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.

Upvotes: 2

Related Questions