Unnecessary DRY
Comments(0) I stumbled over an annoying set of refactorings this morning.
First, the site Refactor My Code, which I’ve heard about and seen a couple of times but never really investigated. One of the refactorings showed DRY taken a bit too literally, and in a way which I see all too often. The original poster quite rightly wants to reduce his ActionMailer code, but how to do it?
The first responder has the best solution: refactor the core of the methods to a standard ‘send mail’ method. The only improvement I would make is to add a yield, thus allowing customisation via a block. Using this method incurs one line per a new variation (though this could be more if a block is used).
The next two refactorings are not so good. One uses define_method to add the methods dynamically. The other uses method_missing. In both cases this is misguided.
Using define_method is pointless: it results in marginally fewer lines of code than the previous solution, but loses flexibility.
Use method_missing is the same — using Ruby tricks where none are needed. Since templates need to be defined for each email to be sent, there’s no real advantage to automagically processing with method_missing, since you’ll know which emails are going to be sent by the templates you’ve defined.
The define_method refactoring was voted higher than the first refactoring, which only goes to prove that programmers often prefer being tricky to being practical. Ruby programmers unfortunately take DRY to extremes, and this is just another case in point.