Recursive grid traveling algorithm

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
4
down vote

favorite












I'm trying to come up with a solution to a coding challenge on Kattis, where there is a 2D grid filled with x's and o's, and the goal is to figure out whether or not a given starting coordinate can reach a given end coordinate by traveling up, down, left, or right. Only the o's are navigable, and the width and height of the grid can each be up to 1000 characters.



Here is an example grid:



xoxxx
ooxxo
oxxxo
ooooo
oxxxx
ooooo


I'll define this as



std::vector<std::string> space;


For this space, e.g., space[0][1] can reach space[5][4].



Note: This isn't quite the problem. I've generalized it a bit because there are some nuances that are a bit irrelevant to efficiency. Here's the actual problem.



I've taken a recursive approach to this problem, and my program passes the first 22 out of the 25 total test cases, before exceeding the 1-second time limit. The algorithm that I thought of was to keep checking whether the next valid coordinate could reach the end. The base case occurs when the starting coordinate is the ending coordinate, and it signifies that the problem's starting point can reach the ending point. If there is no valid next location then the current path cannot reach the ending point. If all the paths reach a dead end then that signifies that the problem's starting point cannot travel to the ending point.



Here are the brains of the program (specifically canTravel):



struct Point 
int x, y;
friend bool operator==(const Point &p1, const Point &p2);
;

bool operator==(const Point &p1, const Point &p2)
return p1.x == p2.x && p1.y == p2.y;


std::vector<Point> memo; // take note of the visited locations to avoid revisiting them

bool memoContains(const Point &p1)
return std::find_if(memo.begin(), memo.end(), [&](const Point &p) return p == p1; ) != memo.end();


bool canTravel(const std::vector<std::string> &space, const Point &start, const Point &end) down


Why is this slow/inefficient? How can it be improved? Can a recursive approach be used here or is an iterative one preferable?







share|improve this question



























    up vote
    4
    down vote

    favorite












    I'm trying to come up with a solution to a coding challenge on Kattis, where there is a 2D grid filled with x's and o's, and the goal is to figure out whether or not a given starting coordinate can reach a given end coordinate by traveling up, down, left, or right. Only the o's are navigable, and the width and height of the grid can each be up to 1000 characters.



    Here is an example grid:



    xoxxx
    ooxxo
    oxxxo
    ooooo
    oxxxx
    ooooo


    I'll define this as



    std::vector<std::string> space;


    For this space, e.g., space[0][1] can reach space[5][4].



    Note: This isn't quite the problem. I've generalized it a bit because there are some nuances that are a bit irrelevant to efficiency. Here's the actual problem.



    I've taken a recursive approach to this problem, and my program passes the first 22 out of the 25 total test cases, before exceeding the 1-second time limit. The algorithm that I thought of was to keep checking whether the next valid coordinate could reach the end. The base case occurs when the starting coordinate is the ending coordinate, and it signifies that the problem's starting point can reach the ending point. If there is no valid next location then the current path cannot reach the ending point. If all the paths reach a dead end then that signifies that the problem's starting point cannot travel to the ending point.



    Here are the brains of the program (specifically canTravel):



    struct Point 
    int x, y;
    friend bool operator==(const Point &p1, const Point &p2);
    ;

    bool operator==(const Point &p1, const Point &p2)
    return p1.x == p2.x && p1.y == p2.y;


    std::vector<Point> memo; // take note of the visited locations to avoid revisiting them

    bool memoContains(const Point &p1)
    return std::find_if(memo.begin(), memo.end(), [&](const Point &p) return p == p1; ) != memo.end();


    bool canTravel(const std::vector<std::string> &space, const Point &start, const Point &end) down


    Why is this slow/inefficient? How can it be improved? Can a recursive approach be used here or is an iterative one preferable?







    share|improve this question























      up vote
      4
      down vote

      favorite









      up vote
      4
      down vote

      favorite











      I'm trying to come up with a solution to a coding challenge on Kattis, where there is a 2D grid filled with x's and o's, and the goal is to figure out whether or not a given starting coordinate can reach a given end coordinate by traveling up, down, left, or right. Only the o's are navigable, and the width and height of the grid can each be up to 1000 characters.



      Here is an example grid:



      xoxxx
      ooxxo
      oxxxo
      ooooo
      oxxxx
      ooooo


      I'll define this as



      std::vector<std::string> space;


      For this space, e.g., space[0][1] can reach space[5][4].



      Note: This isn't quite the problem. I've generalized it a bit because there are some nuances that are a bit irrelevant to efficiency. Here's the actual problem.



      I've taken a recursive approach to this problem, and my program passes the first 22 out of the 25 total test cases, before exceeding the 1-second time limit. The algorithm that I thought of was to keep checking whether the next valid coordinate could reach the end. The base case occurs when the starting coordinate is the ending coordinate, and it signifies that the problem's starting point can reach the ending point. If there is no valid next location then the current path cannot reach the ending point. If all the paths reach a dead end then that signifies that the problem's starting point cannot travel to the ending point.



      Here are the brains of the program (specifically canTravel):



      struct Point 
      int x, y;
      friend bool operator==(const Point &p1, const Point &p2);
      ;

      bool operator==(const Point &p1, const Point &p2)
      return p1.x == p2.x && p1.y == p2.y;


      std::vector<Point> memo; // take note of the visited locations to avoid revisiting them

      bool memoContains(const Point &p1)
      return std::find_if(memo.begin(), memo.end(), [&](const Point &p) return p == p1; ) != memo.end();


      bool canTravel(const std::vector<std::string> &space, const Point &start, const Point &end) down


      Why is this slow/inefficient? How can it be improved? Can a recursive approach be used here or is an iterative one preferable?







      share|improve this question













      I'm trying to come up with a solution to a coding challenge on Kattis, where there is a 2D grid filled with x's and o's, and the goal is to figure out whether or not a given starting coordinate can reach a given end coordinate by traveling up, down, left, or right. Only the o's are navigable, and the width and height of the grid can each be up to 1000 characters.



      Here is an example grid:



      xoxxx
      ooxxo
      oxxxo
      ooooo
      oxxxx
      ooooo


      I'll define this as



      std::vector<std::string> space;


      For this space, e.g., space[0][1] can reach space[5][4].



      Note: This isn't quite the problem. I've generalized it a bit because there are some nuances that are a bit irrelevant to efficiency. Here's the actual problem.



      I've taken a recursive approach to this problem, and my program passes the first 22 out of the 25 total test cases, before exceeding the 1-second time limit. The algorithm that I thought of was to keep checking whether the next valid coordinate could reach the end. The base case occurs when the starting coordinate is the ending coordinate, and it signifies that the problem's starting point can reach the ending point. If there is no valid next location then the current path cannot reach the ending point. If all the paths reach a dead end then that signifies that the problem's starting point cannot travel to the ending point.



      Here are the brains of the program (specifically canTravel):



      struct Point 
      int x, y;
      friend bool operator==(const Point &p1, const Point &p2);
      ;

      bool operator==(const Point &p1, const Point &p2)
      return p1.x == p2.x && p1.y == p2.y;


      std::vector<Point> memo; // take note of the visited locations to avoid revisiting them

      bool memoContains(const Point &p1)
      return std::find_if(memo.begin(), memo.end(), [&](const Point &p) return p == p1; ) != memo.end();


      bool canTravel(const std::vector<std::string> &space, const Point &start, const Point &end) down


      Why is this slow/inefficient? How can it be improved? Can a recursive approach be used here or is an iterative one preferable?









      share|improve this question












      share|improve this question




      share|improve this question








      edited Jan 27 at 3:20
























      asked Jan 27 at 3:08









      Archie Gertsman

      2291310




      2291310




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          1
          down vote













          What is killing your performance is memoContains. It turns your algorithm from O(n) into O(n^2).



          The better solution is to create a second array of the same size as your input, where you mark each cell visited. Then you can do the check in O(1).



          Next, you need to implement a short circuit for your logic. If right is true, there is no point in computing the other directions.



          Why do you define right and the other three bools as references?



          There are some more tricks you can apply, for example to avoid checking for out-of-bounds indexing by simply making the array one cell larger on all sides, and filling those positions with x.



          Finally, regarding recursion. It's a nice way of describing the algorithm. You might get better performance if you use a stack instead. But the only way to know for sure is to try it out and time it. :)



          Oh, and one more thing:



          std::vector<std::string> space;


          Is highly inefficient. Your data is all over the memory because the data for each array is allocated independently. You should probably make this a single vector (or string if you must) that you index as row * width + column. Note that now you can index neighboring cells using simple pointer arithmetic (add 1 to the pointer to access the cell on the right, for example).






          share|improve this answer























          • Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
            – Archie Gertsman
            Jan 27 at 3:43










          • @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
            – Cris Luengo
            Jan 27 at 3:47






          • 1




            When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
            – 1201ProgramAlarm
            Jan 27 at 4:27










          • And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
            – Cris Luengo
            Jan 27 at 4:37










          • @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
            – Archie Gertsman
            Jan 30 at 16:03











          Your Answer




          StackExchange.ifUsing("editor", function ()
          return StackExchange.using("mathjaxEditing", function ()
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          );
          );
          , "mathjax-editing");

          StackExchange.ifUsing("editor", function ()
          StackExchange.using("externalEditor", function ()
          StackExchange.using("snippets", function ()
          StackExchange.snippets.init();
          );
          );
          , "code-snippets");

          StackExchange.ready(function()
          var channelOptions =
          tags: "".split(" "),
          id: "196"
          ;
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function()
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled)
          StackExchange.using("snippets", function()
          createEditor();
          );

          else
          createEditor();

          );

          function createEditor()
          StackExchange.prepareEditor(
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: false,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          );



          );








           

          draft saved


          draft discarded


















          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186107%2frecursive-grid-traveling-algorithm%23new-answer', 'question_page');

          );

          Post as a guest






























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          1
          down vote













          What is killing your performance is memoContains. It turns your algorithm from O(n) into O(n^2).



          The better solution is to create a second array of the same size as your input, where you mark each cell visited. Then you can do the check in O(1).



          Next, you need to implement a short circuit for your logic. If right is true, there is no point in computing the other directions.



          Why do you define right and the other three bools as references?



          There are some more tricks you can apply, for example to avoid checking for out-of-bounds indexing by simply making the array one cell larger on all sides, and filling those positions with x.



          Finally, regarding recursion. It's a nice way of describing the algorithm. You might get better performance if you use a stack instead. But the only way to know for sure is to try it out and time it. :)



          Oh, and one more thing:



          std::vector<std::string> space;


          Is highly inefficient. Your data is all over the memory because the data for each array is allocated independently. You should probably make this a single vector (or string if you must) that you index as row * width + column. Note that now you can index neighboring cells using simple pointer arithmetic (add 1 to the pointer to access the cell on the right, for example).






          share|improve this answer























          • Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
            – Archie Gertsman
            Jan 27 at 3:43










          • @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
            – Cris Luengo
            Jan 27 at 3:47






          • 1




            When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
            – 1201ProgramAlarm
            Jan 27 at 4:27










          • And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
            – Cris Luengo
            Jan 27 at 4:37










          • @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
            – Archie Gertsman
            Jan 30 at 16:03















          up vote
          1
          down vote













          What is killing your performance is memoContains. It turns your algorithm from O(n) into O(n^2).



          The better solution is to create a second array of the same size as your input, where you mark each cell visited. Then you can do the check in O(1).



          Next, you need to implement a short circuit for your logic. If right is true, there is no point in computing the other directions.



          Why do you define right and the other three bools as references?



          There are some more tricks you can apply, for example to avoid checking for out-of-bounds indexing by simply making the array one cell larger on all sides, and filling those positions with x.



          Finally, regarding recursion. It's a nice way of describing the algorithm. You might get better performance if you use a stack instead. But the only way to know for sure is to try it out and time it. :)



          Oh, and one more thing:



          std::vector<std::string> space;


          Is highly inefficient. Your data is all over the memory because the data for each array is allocated independently. You should probably make this a single vector (or string if you must) that you index as row * width + column. Note that now you can index neighboring cells using simple pointer arithmetic (add 1 to the pointer to access the cell on the right, for example).






          share|improve this answer























          • Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
            – Archie Gertsman
            Jan 27 at 3:43










          • @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
            – Cris Luengo
            Jan 27 at 3:47






          • 1




            When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
            – 1201ProgramAlarm
            Jan 27 at 4:27










          • And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
            – Cris Luengo
            Jan 27 at 4:37










          • @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
            – Archie Gertsman
            Jan 30 at 16:03













          up vote
          1
          down vote










          up vote
          1
          down vote









          What is killing your performance is memoContains. It turns your algorithm from O(n) into O(n^2).



          The better solution is to create a second array of the same size as your input, where you mark each cell visited. Then you can do the check in O(1).



          Next, you need to implement a short circuit for your logic. If right is true, there is no point in computing the other directions.



          Why do you define right and the other three bools as references?



          There are some more tricks you can apply, for example to avoid checking for out-of-bounds indexing by simply making the array one cell larger on all sides, and filling those positions with x.



          Finally, regarding recursion. It's a nice way of describing the algorithm. You might get better performance if you use a stack instead. But the only way to know for sure is to try it out and time it. :)



          Oh, and one more thing:



          std::vector<std::string> space;


          Is highly inefficient. Your data is all over the memory because the data for each array is allocated independently. You should probably make this a single vector (or string if you must) that you index as row * width + column. Note that now you can index neighboring cells using simple pointer arithmetic (add 1 to the pointer to access the cell on the right, for example).






          share|improve this answer















          What is killing your performance is memoContains. It turns your algorithm from O(n) into O(n^2).



          The better solution is to create a second array of the same size as your input, where you mark each cell visited. Then you can do the check in O(1).



          Next, you need to implement a short circuit for your logic. If right is true, there is no point in computing the other directions.



          Why do you define right and the other three bools as references?



          There are some more tricks you can apply, for example to avoid checking for out-of-bounds indexing by simply making the array one cell larger on all sides, and filling those positions with x.



          Finally, regarding recursion. It's a nice way of describing the algorithm. You might get better performance if you use a stack instead. But the only way to know for sure is to try it out and time it. :)



          Oh, and one more thing:



          std::vector<std::string> space;


          Is highly inefficient. Your data is all over the memory because the data for each array is allocated independently. You should probably make this a single vector (or string if you must) that you index as row * width + column. Note that now you can index neighboring cells using simple pointer arithmetic (add 1 to the pointer to access the cell on the right, for example).







          share|improve this answer















          share|improve this answer



          share|improve this answer








          edited Jan 27 at 3:47


























          answered Jan 27 at 3:36









          Cris Luengo

          1,877215




          1,877215











          • Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
            – Archie Gertsman
            Jan 27 at 3:43










          • @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
            – Cris Luengo
            Jan 27 at 3:47






          • 1




            When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
            – 1201ProgramAlarm
            Jan 27 at 4:27










          • And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
            – Cris Luengo
            Jan 27 at 4:37










          • @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
            – Archie Gertsman
            Jan 30 at 16:03

















          • Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
            – Archie Gertsman
            Jan 27 at 3:43










          • @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
            – Cris Luengo
            Jan 27 at 3:47






          • 1




            When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
            – 1201ProgramAlarm
            Jan 27 at 4:27










          • And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
            – Cris Luengo
            Jan 27 at 4:37










          • @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
            – Archie Gertsman
            Jan 30 at 16:03
















          Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
          – Archie Gertsman
          Jan 27 at 3:43




          Oh whoops I actually added that short circuiting but accidentally pasted my previous version of this function. I'll give everything else a shot, though.
          – Archie Gertsman
          Jan 27 at 3:43












          @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
          – Cris Luengo
          Jan 27 at 3:47




          @ArchieGertsman: I have edited one more time, I keep finding more things to comment on. :)
          – Cris Luengo
          Jan 27 at 3:47




          1




          1




          When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
          – 1201ProgramAlarm
          Jan 27 at 4:27




          When using the second array to hold cells that have already been visited, you can set the initial values so that all the 'x' cells are marked visited. Then you won't need to check space for 'o'.
          – 1201ProgramAlarm
          Jan 27 at 4:27












          And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
          – Cris Luengo
          Jan 27 at 4:37




          And since you are copying the input into a larger array anyway, you can use 0 and 1 instead of 'o' and 'x', and use any of the other bits in the byte to mark a cell as visited. Using less memory makes it more efficient, the requited bit operations are trivial.
          – Cris Luengo
          Jan 27 at 4:37












          @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
          – Archie Gertsman
          Jan 30 at 16:03





          @CrisLuengo I've implemented your first and last suggestions, and it's still going over the time limit. Could you please take a look at the updated code? pastebin.com/NHY9dx0g
          – Archie Gertsman
          Jan 30 at 16:03













           

          draft saved


          draft discarded


























           


          draft saved


          draft discarded














          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186107%2frecursive-grid-traveling-algorithm%23new-answer', 'question_page');

          );

          Post as a guest













































































          Popular posts from this blog

          Chat program with C++ and SFML

          Function to Return a JSON Like Objects Using VBA Collections and Arrays

          Will my employers contract hold up in court?