# Can I simplify this recursive grid search?

I've written a recursive method for fetching grid neighbours in a symmetrical 2-dimensional grid, the problem is that I've found myself duplicating the code more or less into an overload to prevent my list from re-initializing everytime the method calls on itself.

I'm guessing there are ways to get around this that doesn't involve code duplication. Any help is appreciated.

public List<Cell<int>> GetNeighbours(Cell<int> cell, Direction direction, int neighboursToGet)
{
var neighbours = new List<Cell<int>>();

if (neighboursToGet > 0)
{
int x = 0;
int y = 0;
Cell<int> neighbour;

switch (direction)
{
case Direction.Left:
x = -1;
break;
case Direction.Right:
x = 1;
break;
case Direction.Up:
y = -1;
break;
case Direction.Down:
y = 1;
break;
}

neighbour = cells[cell.Position.Row + y, cell.Position.Column + x];
GetNeighbours(neighbour, direction, neighboursToGet - 1, neighbours);
}
return neighbours;
}

public List<Cell<int>> GetNeighbours(Cell<int> cell, Direction direction, int neighboursToGet, List<Cell<int>> neighbours)
{
if (neighboursToGet > 0)
{
int x = 0;
int y = 0;
Cell<int> neighbour;

switch (direction)
{
case Direction.Left:
x = -1;
break;
case Direction.Right:
x = 1;
break;
case Direction.Up:
y = -1;
break;
case Direction.Down:
y = 1;
break;
}

neighbour = cells[cell.Position.Row + y, cell.Position.Column + x];
GetNeighbours(neighbour, direction, neighboursToGet - 1, neighbours);
}
return neighbours;
}


It seems to me that most of the body of the first GetNeighbours is extraneous. Wouldn't this be enough ?

public List<Cell<int>> GetNeighbours(Cell<int> cell, Direction direction, int neighboursToGet)
{
var neighbours = new List<Cell<int>>();

GetNeighbours(cell, direction, neighboursToGet, neighbours);

return neighbours;
}


Also, it is redundant to accept neighbours as a read/write parameter and to return it. However, if you are going to do it, you could as well write:

public List<Cell<int>> GetNeighbours(Cell<int> cell, Direction direction, int neighboursToGet)
{
return GetNeighbours(cell, direction, neighboursToGet, new List<Cell<int>>());
}

• Good god.. You're absolutely right, I feel so dumbfounded right now :D .. – Overly Excessive May 2 '14 at 15:43

As a first step, I'd change this implementation:

public List<Cell<int>> GetNeighbours(Cell<int> cell,
Direction direction,
int neighboursToGet)


To call into this one:

public List<Cell<int>> GetNeighbours(Cell<int> cell,
Direction direction,
int neighboursToGet,
List<Cell<int>> neighbours)


But @Zoyd was faster to spot it ;)

Bottom line is, when you find yourself hitting Ctrl+C, a big shiny red flag should be automatically raised in your mind before you even have time to hit Ctrl+V - if you're copying code, you're doing it wrong.

I like that you're using an enum for your Direction. However you could also tackle the switch like this:

var directionalOffsets = new Dictionary<Direction, Func<Point, Point>>
{
{ Direction.Left, point => new Point(point.X - 1, point.Y) },
{ Direction.Right, point => new Point(point.X + 1, point.Y) },
{ Direction.Up, point => new Point(point.X, point.Y - 1) },
{ Direction.Down, point => new Point(point.X, point.Y + 1) }
};


Which, assuming cell.Position can be refactored into a Point, means if (neighboursToget > 0)...

var position = directionalOffsets[direction](cell.Position);


Then neighbour could be retrieved like this:

var neighbour = cells[position.Y, position.X];


Now it looks like cells has its dimensions reversed. I'd be more instinctively expecting this:

var neighbour = cells[position.X, position.Y];