Search code examples
c#algorithmrecursionknights-tour

Knights Tour recursive C# Im getting something wrong in my way of doing the stuff


class Knight
{
    public static readonly double LegalDistance = Math.Sqrt(5);
    public Stack<Field> Steps { get; set; }

    private static readonly List<Field> board = Board.GameBoard;
    private static List<Field> fields;       

    private static readonly Random random = new Random();
    private static readonly object synLock = new object(); 

    public Knight(Field initial)
    {
        Steps = new Stack<Field>();
        Steps.Push(initial);
    }

    public void Move()
    {
        Field destination = Choose();

        if (destination == null)
        {
            return;
        }

        Console.WriteLine("Moving from " + GetPosition().GetFieldName() + " to " + destination.GetFieldName());
        Steps.Push(destination);
    }

    public Field Back()
    {
        Field from = Steps.Pop();
        Console.WriteLine("Moving back from " + from.GetFieldName() + " to " + GetPosition().GetFieldName());

        return from;
    }

    public Field Choose()
    {
        List<Field> legalMoves = Behaviour();

        legalMoves.RemoveAll(field => Steps.Contains(field, new FieldValueComparer()));

        if (legalMoves.Count == 0)
        {
            return null;
        }

        Field theChoosenOne;

        int index;
        lock (synLock)
        {
            index = random.Next(0, legalMoves.Count);
        }

        theChoosenOne = legalMoves.ElementAt(index);


        return theChoosenOne;
    }

    private List<Field> Behaviour()
    {
        fields = new List<Field>();
        fields.AddRange(board);

        for (int i = fields.Count - 1; i >= 0; i--)
        {
            double actualDistance = fields[i].GetDistance(GetPosition());
            if (!actualDistance.Equals(LegalDistance))
            {
                fields.Remove(fields[i]);
            }
        }

        return fields;
    }
    public List<Field> GetSteps()
    {
        return Steps.ToList();
    }

    public Field GetPosition()
    {
        return Steps.Peek();
    }
}

So this is how I'd do the stuff. The problem is, I am missing some key functionality, because on low given stepcount it backtracks to the start, on high stepcount, it causes StackOverFlow.

Here are some other functions to let you understand what I want to do: Calculating distance:

public double GetDistance(Field other)
{
       return Math.Sqrt(Math.Pow(other.X - X, 2) + Math.Pow(other.Y - Y, 2));
}

Finding the path:

class PathFinder
    {
        public static void FindPath(Knight knight)
        {

            if (knight.Steps.Count != 20)
            {
                knight.Move();

                FindPath(knight);

                knight.Back();
            }
        }
    }

Solution

  • Your path search is essentially random walk. On large board, this may take a while anyway.

    Now about StackOverflow: notice that you don't push anything on Move() when there are no places to go. So, on recursive call to FindPath() there still will be the same knight.Steps.Count, the same position, the same null return on Choose()... and so on, until you're out of stack space.

    Obvious fix would be to add bool return value to Move() indicating if there was any move. Unless there is actual reason behind using random moves, more deterministic search algorithm is recommended.