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.
), I opened the 32-bit version of CLR profiler instead and did my analysis.
Jamie Dixon's Home 