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

custom equals when creating an atom #79

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

Conversation

AlexSugak
Copy link

This will allow Atom.create clients to provide a custom implementation of equality check explicitly, instead of having an equals method on the value.

This can be useful when we want to have the atom's value as a serializable POJO that could not have any function members.

@blacktaxi
Copy link
Member

Hey, thanks for working on this!

Could you please provide a use case that you're trying to solve with this? In general I'm in favor of making equality configurable for atoms, but I would prefer if it was done on at a higher abstraction layer, possibly. I think it's easy to use custom equality with POJOs in a wrong way, so I'd like to better understand what do you want to achieve here.

Another question is about lensing into a JsonAtom with custom equality. What is the expected behavior here?

@AlexSugak
Copy link
Author

AlexSugak commented Jan 21, 2021

Hi!

The use case is that I want to have a custom equality check for the atom value and at the same time for it to be a serializable POJO (no functions as fields), therefore I cannot use an equals function on the value.

I see that in many places we already pass the equals function as a parameter when constructing atoms, so this change seems logical.

As for lensed atoms, one of the tests I've added shows the behavior which I think is OK (derived atom only updating when parent atom with custom equality updates).
It would be nice to have the overload of view and lens function that accepts custom equals function too, but it is hard to extend it without introducing breaking changes.

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.

None yet

2 participants