Can I simplify this recursive grid search?

Question

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];
                neighbours.Add(neighbour);
                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];
                neighbours.Add(neighbour);
                GetNeighbours(neighbour, direction, neighboursToGet - 1, neighbours);
            }
            return neighbours;
        }

Answer 1

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>>());
}

Answer 2

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];