Visual Studio 2010 Extract Method

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:

image

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:

image

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.

Advertisements

One Response to Visual Studio 2010 Extract Method

  1. 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.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: