Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use same depth-first search for generate and solve #3

Merged
merged 39 commits into from
Feb 26, 2014

Conversation

chrishunt
Copy link
Member

Fixes #2
SJO ✈️ PDX

generate-maze and solve-maze use an identical depth-first search. This pulls the algorithm into search-maze and lets each specify a different finish condition.

  • We are done generating the maze when all locations are visited.
  • We are done solving the maze when we've reached the solved location.

In order to see the bits that these two functions had in common, there's also some function renaming.

(defn generate-maze [maze]
  (search-maze (merge maze {:finished-fn visited?})))

(defn solve-maze [maze]
  (search-maze (merge maze {:finished-fn solved?})))

@@ -9,7 +9,7 @@
:when (not= (.abs js/Math dx) (.abs js/Math dy))]
[(+ x dx) (+ y dy)])))

(defn unvisited-neighbors [location visited size]
(defn unvisited-neighbors [location {:keys [visited size]}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pass the last two as a map but not the first?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar question on other fns below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a weird choice and I'm happy to change if we want.

It seems like we are passing around a bag of bits to a lot of functions. These all seem related to the state of the maze. That last argument is that full bag of bits and we're plucking what we want out of it.

The first argument is the location that we are checking against the maze state. It didn't seem right to stick it in the same bag since it's not related to the maze like the others.

Super happy to tidy this up in another PR. It's pretty inconsistent. There are a few untouched functions that are different yet.

@r00k
Copy link
Contributor

r00k commented Feb 26, 2014

Great change overall.

Good to 🚢

Chris Hunt added 24 commits February 25, 2014 19:54
We cannot use recur in solve-maze because the canvas does not get
updated between iterations. For consistency, it makes sense to not use
recur here either.
This is more clear because we are randomly picking from
unvisited-neighbors
This will be nil already if not provided
This will help merge common behavior into a shared function.
merge with the rcall search-mze
Chris Hunt added 15 commits February 25, 2014 19:55
random-reachable-neighbor works for both generate-maze and solve-maze so
there's no need to use the more general random-unvisited-neighbor for
generate-maze.
This is a clearer definition than an empty path.
In every case, we are going to want to start searching from [0 0]. Let's
make it the default instead of passing it in everywhere.
This is the only function that mentions a 'grid' and it's confusing.
This function really returns all walls for the maze.
The function is only used once and the name isn't very clear. It makes
more sense to move this difference into all-walls-without-doors.
chrishunt added a commit that referenced this pull request Feb 26, 2014
Use same depth-first search for generate and solve
@chrishunt chrishunt merged commit c551182 into master Feb 26, 2014
@chrishunt chrishunt deleted the de-dup-search-and-solve branch February 26, 2014 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplication in solve-maze and generate-maze
2 participants