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

sorted todomvc using js/Date #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

sorted todomvc using js/Date #5

wants to merge 1 commit into from

Conversation

usernolan
Copy link

Naive implementation of sorting the todos.

@usernolan
Copy link
Author

This was a pretty simple approach. I can't tell if there are any trade offs between sorting in context (like this solution) and sorting in the rule that produces visible-todos. Do you think it would be easier to implement a "sort" enum like the visibility-filter i.e. hold a sort-enum fact and conditionally sort in the rule?

@alex-dixon
Copy link
Contributor

I think that's a great way to do it actually. I've been relatively dogmatic about doing everything with rules and I think this shows that's not always necessary. Thanks for sharing this.

The rules engine should be able to produce a sorted list though. For this case I think the best solution is a custom accumulator. This will produce a list that we (currently) can't pattern match on, but it could be output for public consumption and still be useful.

The accumulators namespace shows a couple efforts to make custom accumulator functions. Clara has designed a great interface for this. We should be able to maintain and return a list of facts and return it, and provide parameters to define what to sort by (value usually) and what to return. Note we can't currently pass bound variables (anything with ?) as arguments. :(

The approach with visible todos was to get accumulate all eids of todos that are visible and then pass that list of eids to entities in order to accumulate all matches for each eid into a list with the same order as the list of eids. Doing so is convenient for view consumption and doesn't require a view to know about sorting.

Here's somewhat of a hybrid approach we took in the fullstack example that prevents the view from having to perform or know about sort logic: https://github.com/CoNarrative/precept/blob/master/examples/fullstack/src/cljs/fullstack/rules.cljs#L137

@usernolan
Copy link
Author

That definitely seems like a better, less naive solution. I think the sort order should be decoupled from the views (although leaving this as an option isn't a bad thing), and rather provide a parameterized accumulator like you mention/directly expose the expressiveness in Clara.

I haven't been able to work much with accumulators--if you want to give me a 30 sec intro to repl'ing with them, I'd be happy to PR some examples to the docs to lower that specific barrier of entry. The reason this might be helpful is that, while Precept relieves the pain point of structuring and restructuring state in traditional Redux/re-frame (which is a huge strength), it's important to understand how accumulating the view of the world in a particular shape (e.g. sort) should be done. Even if the one-off solutions will always exist (and probably even be pretty ok, like this little 3 line change, given its clj/s).

Anyway, sorry to continue bothering you on the todomvc example! It's been great getting experience with the framework.

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.

2 participants