Ruby Refactorings
I think I accidentally discovered two new refactorings today, Extract Mixin and Replace Mixin With Class. (I'll make detailed writeups in a day or two.) This morning I found myself in central Gothenburg with nothing to do for an hour before a meeting, so I ducked into a cafe, had a cup of tea, and began refactoring Test::Unit::XML.
If you have had a look at the code in the assertions mixin, xml_assertions.rb, in version 0.1.4, you'll notice that it is fairly heavyweight, weighing in at more than 220 lines of code. This is a fairly large chunk of code, and all of it is for a single assertion, assert_xml_equal. A big part of that code is for comparing different types of REXML nodes, REXML::Element, REXML::Text, and so on.
The node comparison code was split into fairly small methods, so it wasn't too shabby, it was just that it was located in the wrong place. It made sense to move that code to a class of its own.
I did consider moving it to a mixin module, but in the end I decided against it. The main reason is that the XML assertions in Test::Unit::XML are mixed in with the Test::Unit::TestCase class. I don't want to pollute Test::Unit::TestCase with a lot of methods that don't need to be there.
However, extracting the comparison methods into a mixin of its own was a logical first step. I created a mixin module, moved the XML comparison methods there, added an include statement to the xml_assertions.rb file, and ran my test suite. Everything worked.
The second step was to replace the new comparison mixin with a class. First, I wrote a comparison class, and included the comparison mixin. Second, I changed the XML assertion module in xml_assertions.rb so that it instantiated the comparison class and called its methods instead of using the comparison mixins directly. Third, I ran the tests, and they worked. Fourth, I removed the include statement that included the comparison mixins in the assertions module. Step five was to move the comparison methods into the comparison class and remove the comparison mixin. Finally, I ran the tests again, and everything was OK.
If you have had a look at the code in the assertions mixin, xml_assertions.rb, in version 0.1.4, you'll notice that it is fairly heavyweight, weighing in at more than 220 lines of code. This is a fairly large chunk of code, and all of it is for a single assertion, assert_xml_equal. A big part of that code is for comparing different types of REXML nodes, REXML::Element, REXML::Text, and so on.
The node comparison code was split into fairly small methods, so it wasn't too shabby, it was just that it was located in the wrong place. It made sense to move that code to a class of its own.
I did consider moving it to a mixin module, but in the end I decided against it. The main reason is that the XML assertions in Test::Unit::XML are mixed in with the Test::Unit::TestCase class. I don't want to pollute Test::Unit::TestCase with a lot of methods that don't need to be there.
However, extracting the comparison methods into a mixin of its own was a logical first step. I created a mixin module, moved the XML comparison methods there, added an include statement to the xml_assertions.rb file, and ran my test suite. Everything worked.
The second step was to replace the new comparison mixin with a class. First, I wrote a comparison class, and included the comparison mixin. Second, I changed the XML assertion module in xml_assertions.rb so that it instantiated the comparison class and called its methods instead of using the comparison mixins directly. Third, I ran the tests, and they worked. Fourth, I removed the include statement that included the comparison mixins in the assertions module. Step five was to move the comparison methods into the comparison class and remove the comparison mixin. Finally, I ran the tests again, and everything was OK.
Comments