I am reading specific parts of Martin Fowlers refactoring book again (the areas I was not clear about the first time round). I am looking at the Extract Method chapter at the moment. I can understand why Extract Method is beneficial; for example:
1) Inheritance and overriding
2) Clarity for the user of the class
Say I have some code like the below ( This is a DDD domain Service). Is this a candidate for Extract Method?:
public IEnumerable<KeyValuePair<int, int>> CalculateDenominationsFor(int cost)
{
var target = cost;
foreach (var denomination in currency.AvailableDenominations.OrderByDescending(a => a))
{
var numberRequired = target / denomination;
if (numberRequired > 0)
{
yield return new KeyValuePair<int, int>(denomination, numberRequired);
}
target = target - (numberRequired * denomination);
}
}
I guess I could extract the following lines of code to methods:
target = target - (numberRequired * denomination);
and:
yield return new KeyValuePair<int, int>(denomination, numberRequired);
The concerns I have about my two ideas above are:
1) They would be private methods so no benefit to the caller.
2) The class is currently sealed so no Inheritance benefits.
Is there any guidance available stating when to use Extract Method? Am I overthinking this? I am trying to apply this principle of least astonishment and find myself overthinking a lot recently.
var. I had to read it three times to see all that variables are integers, and the code makes an integer division. Do yourself a favor and writeintinstead ofvar, that is not even more typing.target = target % denomination, i.e. the remainder after giving that change. It then becomes clear that every line is integral to the solution, and nothing is worth extracting. (Except maybe fetching and ordering the denominations? Dunno. This method should arguably be called GiveChange(), and be a member of a currency class.)