0

The Knight's Tour Problem: The knight is placed on the first block of an empty board and, moving according to the rules of chess, must visit each square exactly once. Print the path such that it covers all the blocks.

Could not understand why the code is stuck in an infinite loop. wasted hours still no clue. I have the solution but couldn't understand why this particular code is not working.

#include<bits/stdc++.h>
using namespace std;
#define N 8

//possible moves
int rowMove[] = {2,1,-1,-2,-2,-1,1,2};
int colMove[] = {1,2,2,1,-1,-2,-2,-1};


void printBoard(vector<vector<int>> visited)
{
    for(int i=0; i<N; i++)
    {
        for(int j=0;j<N;j++)
            cout<<visited[i][j]<<" ";
        cout<<endl;
    }
    cout<<endl;
}

//check if the given move is valid
bool isValid(vector<vector<int>> visited,int row, int col)
{
    return (row>=0 && row<N && col>=0 && col<N && visited[row][col]==0);
}


bool solveKnight(vector<vector<int>> visited,int row, int col,int move)
{
    //printBoard(visited);
    if(move==N*N)
    {
        printBoard(visited);
        return true;
    }

    else
    {
        for(int i=0;i<8;i++)
        {
            int newRow = row + rowMove[i];
            int newCol = col + colMove[i];

            if(isValid(visited,newRow,newCol))
            {
                move++;
                visited[newRow][newCol] = move;
                if(solveKnight(visited,newRow,newCol,move))
                    return true;
                move--;
                visited[newRow][newCol]=0;
            }
        }
    }
    return false;
}


int main()
{
    vector<vector<int>> visited(N,vector<int>(N,0));
    visited[0][0]=1;
    if(!solveKnight(visited,0,0,1))
        cout<<"not possible";
    return 0;
}
  • 1
    Did you try stepping through your code with a debugger, while investigating the values of variables, at each execution step? – Algirdas Preidžius Sep 24 '19 at 20:44
  • 2
    Yikes. At every step of the recursion, you make 8 recursive calls. You stop when you're 8*8 = 64 levels of recursion deep. That means you'll end up with 8^64 = 2^192 calls to `solveKnight()`. It doesn't look like an infinite loop to me. Just a really *really* inefficient algorithm. – scohe001 Sep 24 '19 at 20:46
  • 3
    Not doing the full review, but this is most certainly wrong: `vector> visited`. You want a reference here. – SergeyA Sep 24 '19 at 20:46
  • @scohe001 similar code but gives result in under 2 sec: https://www.geeksforgeeks.org/the-knights-tour-problem-backtracking-1/ –  Sep 24 '19 at 20:50
  • @scohe001 This is just a normal depth-first search, isn't it? – Rup Sep 24 '19 at 20:51
  • @NikhilSingh "_similar code but gives result in under 2 sec_" At the very least - the "similar" code doesn't copy the whole 2D array, at each invocation, like yours do. – Algirdas Preidžius Sep 24 '19 at 20:53
  • @scohe001 this is actually an OK naive implementation of BSF. Performance could be improved (i.e. by using heuristic to select the next square), but algo-wise it still is OK even in it's current form (my other comment notwithstanding, of course) – SergeyA Sep 24 '19 at 20:57
  • @Rup sure, but I wouldn't expect a DFS to try to copy 64 int's *and* push 9 lines to stdout at every node it hits. – scohe001 Sep 24 '19 at 21:02
  • 1
    Totally unrelated side note: be really careful with the combination of `#include` (include the entire C++ Standard library-that's a lot of smurf you aren't using) and `using namespace std;` (Effectively place everything in the standard namespace in the global namespace). This pollutes the hell out of the global namespace turning your code into a nightmarish landmine of identifiers for you to trip over. – user4581301 Sep 24 '19 at 21:04
  • @scohe001 True, but that's an implementation mistake not a problem with the algorithm. But hey. – Rup Sep 24 '19 at 21:06
  • @Rup "I wouldn't expect a DFS to try to copy 64 int's and push 9 lines to stdout at every node it hits." my bad. that was for debugging purpose. –  Sep 24 '19 at 21:06

1 Answers1

1

There are two things wrong here:

  1. You're copying over the 64-int vector every. single. time.
  2. You're printing 9 lines to stdout every. single. time.

Both of these are extremely expensive operations. And you could recurse up to 8^64 = 2^192 times. That's not an insignificant number.

If you pass your vector by reference and kill the printBoard at the beginning of every recursive call...viola! https://ideone.com/K0DU3q

scohe001
  • 15,110
  • 2
  • 31
  • 51