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 reductions to create lazy sequences #2

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

glv
Copy link
Owner

@glv glv commented Jan 2, 2023

All of the algorithms use lazy-seq and cons to build a lazy sequence of steps in carving out the maze. That process is a little bit fiddly; at least, it's hard for me to remember what should go where when I'm writing something like that from scratch.

But the reductions function manages all of that for you. It's just like reduce, but produces a lazy sequence of all of the intermediate values instead of just the final value. I wasn't aware of reductions when I originally wrote all of this code.

This is a draft because so far I've only converted binary-tree-seq, as it was the easiest. The others have additional intermediate state they pass from step to step; to work with reductions they will have to move that extra state to a field within the grid. (Alternatively, the state value for the reduction could be a tuple of [grid other-state] and the code that uses the lazy seq from reductions would have to know to map first over the sequence before using it. However, I think adding the extra algorithm-specific state to the grid is the better approach; it's part of Clojure's philosophy that maps should have documented required or optional fields for public consumption, but it's perfectly acceptable for code to add extra fields if needed for their own purposes, and consumers should just be prepared to accept and ignore fields that they don't know about.)

There's an additional change that I could also implement while doing this. All of the algorithms come with alg-seq and alg-seq* function pairs. The unary alg-seq function just prepares the grid and then kicks off the process by calling alg-seq*, which always takes more than one argument. I suspect I did it that way because the alg-seq functions are intended to be called from outside this namespace, but the alg-seq* functions should only be called from their corresponding alg-seq function. But I think instead it would make more sense to just have the alg-seq functions support multiple arities, and have their docstrings only document the unary version. Then the unary version would prepare the grid and call reductions to use the binary version.

@glv
Copy link
Owner Author

glv commented Jan 2, 2023

You can see just from the initial commit that this will be an improvement. It only removed two lines of code, but those two lines (lazy-seq and cons) are "related at a distance" in a way similar to loop/recur, so removing them saves a lot of complexity. Plus the remaining code gets simpler in several ways. Modeling the graph-carving process as a reduction is a big win.

Try out the approach of bundling the public and helper functions into one function with multiple arities, rather than two functions.
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.

1 participant