There are many reasons to extract methods in old legacy code. Sometimes you just see a small piece of cohesive code and you ask yourself whether you can name it. If you can, extracting that code to a method is often a decent thing to do.
In the following code, it's easy to see that the while loop could be extracted into a simple method that forms a vector from the lines that we read from a file. It's a decent extraction, but is it high value?
Let's look a little closer.
What is the code doing? Well, up until the point at which we are getting the system properties, we are creating a list of internet addresses (email addresses) from the contents of a file. Is the way that this is done important to a reader? Probably not. We just care that we get those addresses so that we can use them. Let's extract that method:
Okay, things are much better. Now, the code hides an implementation detail. It hides the mechanism that it uses for getting email addresses from a file. Can we go further? Yes, we can. We can do the extraction above in a way that hides the design decision of going to a file to get the email addresses. In the following code, we just don't care where the email addresses come from:
When I talk to people about these, I sometimes call them 'Parnas Extractions' after David Parnas. Back in 1972, he wrote On the Criteria to be used in decomposing systems into modules, the paper the more or less defined the concept of modularity. The key point is that when we use modularity in a system, we should use it to hide design decisions. Frankly, we can waffle all day about whether a particular decision is a design or an implementation decision, but in the end it doesn't matter. It's a continuum, and extracting getEmailAddresses with a filename argument a step along the same direction as extracting it without one, or moving toward having another object provide the email addresses - it delineates a higher value axis of change in the code.
When you are working in legacy code and feel like breaking up large chunks of code, favor Parnas Extractions. They move your code closer toward embodying a better design.
In your first refactor I would have called the method getEmailAddressFromFile. In your second I don't know what I would have called it, but it would have called getEmailAddressFromFile. What is this global/default list of email addresses? What's its purpose? Can we name it?
When refactoring I think less about hiding implementation details and more about programming to a consistent level of abstraction, and making the method body easy to understand. Method name should embody what it does and method body is how it does it. I think you raise a good point about considering at what level of abstraction each design decision belongs, but doesn't that often fall out of aiming for "The method does and it does it by "?
Posted by: Andrew Johnson | January 22, 2013 at 06:06 AM
Glad to have a name for it - is it synonymous with composed method?
When I do this I feel like am coalescing a new object while separating an object's intent from implementation. Which means both are clarified. I like to use it when approaching a refactor in unfamiliar territory.
Posted by: Ben | January 22, 2013 at 08:38 PM
The "long way" simply involves extracting a few tiny things, seeing duplication in the names, then combining them into what you have here. :)
Posted by: J. B. Rainsberger | January 24, 2013 at 12:14 PM