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

Table with row selection: automatically convert to set #6

Open
Frozenlock opened this issue Nov 7, 2015 · 3 comments
Open

Table with row selection: automatically convert to set #6

Frozenlock opened this issue Nov 7, 2015 · 3 comments

Comments

@Frozenlock
Copy link

Would it be possible to just coerce any existing data to a set?

To see the problem:
Run the demo, but with an initial value of an empty vector, instead of nil.

https://github.com/bilus/reagent-reforms/blob/master/examples/controls/src/controls.cljs#L10

@bilus
Copy link
Owner

bilus commented Nov 10, 2015

Thanks a lot for the suggestion.

I'm not quite sure I'd like to complicate the semantics by supporting this but I'm definitely not ruling that out. Is there a reason you prefer a vector instead of a set in your particular application?

@Frozenlock
Copy link
Author

Yes; Many DB will automatically convert sets into other kinds of collection.

In my case, when I store sets, I get back vectors.
It's usually not a problem, because Clojure is built to be able to handles different kind of collections with the same basic functions.

Unfortunately, reforms doesn't like vectors in this particular situation.
This means that data stored in the DBs are incompatible with it.

There's also the case where the user generates the :selected field initial values with a map or filter function. These will return collections which are not sets. If reforms doesn't support them, the user have to convert them manually.

Now, I can understand why you'd want to let this step to the user. However, if reforms can't accept these collections, I think it should warn the user: "The :selected field can only be nil or a set."

@bilus
Copy link
Owner

bilus commented Nov 17, 2015

A set is perfect in this case, esp. for large tables. With a vector we have O(n^2). I would be wary of making it too easy to use a vector because rendering could get sluggish in some cases and the poor user would be none the wiser.

Leaving that aside, it would be possible to create a protocol and make it work for vectors, lists and sets. At the moment I simply have no free time on my hands to do it but if you submit a PR -- fantastic.

BTW. I updated the API reference, it'll go live with the next release -- thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants