Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

First off, mutable structs are evil mutable structs are evil. Next, why expose the array via a property and have an iterator when one or the other will do? And since exposing array fields directly via a property leaks the implementation, let's expose the iterator only instead:

First off, mutable structs are evil. Next, why expose the array via a property and have an iterator when one or the other will do? And since exposing array fields directly via a property leaks the implementation, let's expose the iterator only instead:

First off, mutable structs are evil. Next, why expose the array via a property and have an iterator when one or the other will do? And since exposing array fields directly via a property leaks the implementation, let's expose the iterator only instead:

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Throw in MattDavey MattDavey's dictionary switcher-arounder (oh, yes, note that there is zero reason for you to be using the ref keyword anywhere in your code):

Throw in MattDavey's dictionary switcher-arounder (oh, yes, note that there is zero reason for you to be using the ref keyword anywhere in your code):

Throw in MattDavey's dictionary switcher-arounder (oh, yes, note that there is zero reason for you to be using the ref keyword anywhere in your code):

Source Link
Jesse C. Slicer
  • 14.5k
  • 1
  • 40
  • 54

First off, mutable structs are evil. Next, why expose the array via a property and have an iterator when one or the other will do? And since exposing array fields directly via a property leaks the implementation, let's expose the iterator only instead:

public struct Hexagon
{
 private readonly float height;
 private readonly bool[] hasSides;
 public Hexagon(float height)
 {
 this.height = height;
 this.hasSides = new[] { true, true, true, true, true, true };
 }
 public float Height
 {
 get
 {
 return this.height;
 }
 }
 public bool this[int i]
 {
 get
 {
 if ((i < 0) || (i >= this.hasSides.Length))
 {
 throw new IndexOutOfRangeException("Index is out of bounds for the array.");
 }
 return this.hasSides[i];
 }
 set
 {
 if ((i < 0) || (i >= this.hasSides.Length))
 {
 throw new IndexOutOfRangeException("Index is out of bounds for the array.");
 }
 this.hasSides[i] = value;
 }
 }

So now with that, we can also simplify the CheckBounds method into a more succinct, yet still wholly readable single-line (also made static as I don't see instance variables in play):

private static bool CheckBounds(int x, int y)
{
 return (x >= 0) && (x < RegionDimensions)
 && (y >= 0) && (y < RegionDimensions);
}

Throw in MattDavey's dictionary switcher-arounder (oh, yes, note that there is zero reason for you to be using the ref keyword anywhere in your code):

private static void EvaluateNeighbors(IEnumerable<KeyValuePair<int, Hexagon>> neighbors, Hexagon currentHexagon)
{
 foreach (var neighbor in neighbors)
 {
 if (currentHexagon.Height < neighbor.Value.Height)
 {
 currentHexagon[neighbor.Key] = false;
 }
 else if (MathExtensions.NearlyEqual(currentHexagon.Height, neighbor.Value.Height))
 {
 currentHexagon[neighbor.Key] = false;
 }
 }
}

Finally, the driver looks like this:

var hexagonArray = new Hexagon[RegionDimensions, RegionDimensions];
// Make sure the hexagonArray is filled with valid Hexagons here.
var neighborHexes = new Dictionary<int, Hexagon>();
// Evaluate heights of hexagons and cull sides
for (var x = 0; x < RegionDimensions; x++)
{
 for (var y = 0; y < RegionDimensions; y++)
 {
 // If it's not in bounds it doesn't need its side no matter what.
 if (y % 2 == 0)
 {
 // 0
 if (CheckBounds(x, y + 1))
 {
 neighborHexes.Add(0, hexagonArray[x, y + 1]);
 }
 else
 {
 hexagonArray[x, y][0] = false;
 }
 // 1
 if (CheckBounds(x + 1, y))
 {
 neighborHexes.Add(1, hexagonArray[x + 1, y]);
 }
 else
 {
 hexagonArray[x, y][1] = false;
 }
 // 2
 if (CheckBounds(x, y - 1))
 {
 neighborHexes.Add(2, hexagonArray[x, y - 1]);
 }
 else
 {
 hexagonArray[x, y][2] = false;
 }
 // 3
 if (CheckBounds(x - 1, y - 1))
 {
 neighborHexes.Add(3, hexagonArray[x - 1, y - 1]);
 }
 else
 {
 hexagonArray[x, y][3] = false;
 }
 // 4
 if (CheckBounds(x - 1, y))
 {
 neighborHexes.Add(4, hexagonArray[x - 1, y]);
 }
 else
 {
 hexagonArray[x, y][4] = false;
 }
 // 5
 if (CheckBounds(x - 1, y + 1))
 {
 neighborHexes.Add(5, hexagonArray[x - 1, y + 1]);
 }
 else
 {
 hexagonArray[x, y][5] = false;
 }
 }
 else
 {
 // 0
 if (CheckBounds(x + 1, y + 1))
 {
 neighborHexes.Add(0, hexagonArray[x + 1, y + 1]);
 }
 else
 {
 hexagonArray[x, y][0] = false;
 }
 // 1
 if (CheckBounds(x + 1, y))
 {
 neighborHexes.Add(1, hexagonArray[x + 1, y]);
 }
 else
 {
 hexagonArray[x, y][1] = false;
 }
 // 2
 if (CheckBounds(x + 1, y - 1))
 {
 neighborHexes.Add(2, hexagonArray[x + 1, y - 1]);
 }
 else
 {
 hexagonArray[x, y][2] = false;
 }
 // 3
 if (CheckBounds(x, y - 1))
 {
 neighborHexes.Add(3, hexagonArray[x, y - 1]);
 }
 else
 {
 hexagonArray[x, y][3] = false;
 }
 // 4
 if (CheckBounds(x - 1, y))
 {
 neighborHexes.Add(4, hexagonArray[x - 1, y]);
 }
 else
 {
 hexagonArray[x, y][4] = false;
 }
 // 5
 if (CheckBounds(x, y + 1))
 {
 neighborHexes.Add(5, hexagonArray[x, y + 1]);
 }
 else
 {
 hexagonArray[x, y][5] = false;
 }
 }
 EvaluateNeighbors(neighborHexes, hexagonArray[x, y]);
 neighborHexes.Clear();
 }
}
lang-cs

AltStyle によって変換されたページ (->オリジナル) /