Visual Studio 2010 Extract Method
September 13, 2011 1 Comment
A common joke is that more CPU cycles are wasted than are actually used on a typical desktop computer. Personally, I feel the same way about Visual Studio – I feel like I only use 25% of its capabilities. I ran into a great refactoring problem over the weekend that demonstrated a really cool feature of VS2010 that literally saved me hours of wasted effort.
As you know, I am working on a Windows Phone 7 port of the timeless classic Panzer General. I am have gotten the board set up with icons, but I want to know determine the acceptable movement and line of site for an individual unit. To do so, I would need to loop over all adjacent hexes and see if they were close to the unit – and then loop over those hexes adjacent hexes until I was past an X distance.
I first tried to whip up a recursive method to do this – and after 1 hour or so, I was failing miserably. I then decided to take a different approach – I hand-wrote each loop (hard coded in) like so:
private static List<Hex> GetTotalAdjacentHexes(Hex currentHex, Int32 maxDistance) { List<Hex> hexes = new List<Hex>(); List<Hex> tempList = GetAdjacentHexes(currentHex); foreach (Hex hex in tempList) { hex.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex); var tempList2 = GetAdjacentHexes(hex); foreach (Hex hex2 in tempList2) { hex2.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex2); var tempList3 = GetAdjacentHexes(hex2); foreach (Hex hex3 in tempList3) { hex3.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex3); var tempList4 = GetAdjacentHexes(hex3); foreach (Hex hex4 in tempList4) { hex4.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex4); } } } } return hexes; }
The thing is – it works:
After an hour of trying to hand-write recursion, I went a different direction – parse out each piece and let VS2010 help me. I too the last for..each (hex4…) and put it into a new method
Just following the default naming conventions, I now have 4 methods that look eerily similar:
private static List<Hex> GetTotalAdjacentHexes(Hex currentHex, Int32 maxDistance) { List<Hex> hexes = new List<Hex>(); List<Hex> tempList = GetAdjacentHexes(currentHex); NewMethod(currentHex, hexes, tempList); return hexes; } private static void NewMethod(Hex currentHex, List<Hex> hexes, List<Hex> tempList) { foreach (Hex hex in tempList) { hex.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex); var tempList2 = GetAdjacentHexes(hex); NewMethod2(currentHex, hexes, hex, tempList2); } } private static void NewMethod2(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> tempList2) { foreach (Hex hex2 in tempList2) { hex2.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex2); var tempList3 = GetAdjacentHexes(hex2); NewMethod1(currentHex, hexes, hex, tempList3); } } private static void NewMethod1(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> tempList3) { foreach (Hex hex3 in tempList3) { hex3.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex3); var tempList4 = GetAdjacentHexes(hex3); NewMethod(currentHex, hexes, hex, tempList4); } } private static void NewMethod(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> tempList4) { foreach (Hex hex4 in tempList4) { hex4.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(hex4); } }
You can notice that the 3 final methods all have the same signature. The first thing I did was to make the internal variables sufficiently generic, for example:
private static void NewMethod(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> currentList) { foreach (Hex tempHex in currentList) { tempHex.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(tempHex); } }
I then realized that there are 3 groups of methods. Method Type 1 is for the 1st time through; Method Type 2 is for second to total-1 times through; and Method Type 3 is for the last time.
Concentrating on Type 2 for a second, you can see that they are identical except that 1 calls the other. That seems like a prime candidate for recursion:
private static void NewMethod2(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> currentList) { foreach (Hex tempHex in currentList) { tempHex.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(tempHex); List<Hex> tempList = GetAdjacentHexes(tempHex); NewMethod1(currentHex, hexes, hex, tempList); } }
I changed this line:
NewMethod1(currentHex, hexes, hex, tempList);
To this:
NewMethod2(currentHex, hexes, hex, tempList);
And hit F5 and got this after a delay:
Which is another way of saying out of memory… Basically, there is no way out of the for…each without some kind of counter. So I threw a counter into the method and assigned a magic number of 2 (note the last time though, it only assigns and does not recurse. if I added the counter above the for..each, then the last time though would not assign):
private static void NewMethod2(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> currentList, int counter) { counter++; { foreach (Hex tempHex in currentList) { tempHex.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(tempHex); if (counter < 2) { List<Hex> tempList = GetAdjacentHexes(tempHex); NewMethod2(currentHex, hexes, hex, tempList, counter); } } } }
Sure enough, it worked. I then tackled the 1st method by adding hat counter variable and a Hex parameter that is initially placed with the 1st loop though of the collection and then I renamed the method:
private static List<Hex> GetTotalAdjacentHexes(Hex currentHex, Int32 maxDistance) { List<Hex> hexes = new List<Hex>(); List<Hex> tempList = GetAdjacentHexes(currentHex); foreach (Hex hex in tempList) { GetAdjacentHexes(currentHex, hexes, hex, tempList, 0, maxDistance); } return hexes; } private static void GetAdjacentHexes(Hex currentHex, List<Hex> hexes, Hex hex, List<Hex> currentList, int distanceCounter, int maxDistance) { distanceCounter++; { foreach (Hex tempHex in currentList) { tempHex.MovementCost = HexFactory.CalcualteMovementCost(hex, currentHex.CurrentUnit); hexes.Add(tempHex); if (distanceCounter < maxDistance) { List<Hex> tempList = GetAdjacentHexes(tempHex); GetAdjacentHexes(currentHex, hexes, hex, tempList, distanceCounter, maxDistance); } } }
Voila. A working recursive method (that is begging to be refactored even more). Undoubtedly, VS2010 and its extract method refactoring saved me hours.
Jamie Dixon's Home 
mma universe
I think you have pretty much covered the bases. Could be a couple other thoughts that may also be helpful. Well written piece.