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

Equality definition #7

Open
cvb941 opened this issue Jul 31, 2024 · 3 comments
Open

Equality definition #7

cvb941 opened this issue Jul 31, 2024 · 3 comments

Comments

@cvb941
Copy link

cvb941 commented Jul 31, 2024

Hello,

I have noticed that the equals function on Measure class returns true even when the units of two measurements are different.

This is how Swift defines equality in their Measurement too.

However, I have encountered an issue with this implementation using Compose, where my UI is not recomposed when changing units.

I wonder whether it is correct or not to consider the units in the equality function. Was there a particular decision for the current implementation?

If equals were to include the units in the comparison, we could still use the compareTo as an alternative for the old behaviour.

@pusolito
Copy link
Collaborator

pusolito commented Aug 1, 2024

This is actually by design, to ensure that statements like: 18 * inches == 1.5 * feet, or 500 * milliseconds == 0.5 * seconds. Defining it so only Measures with the same units are equal would break these and violate intuition. It would also make Measures much less useful since you would have to keep track of the units (i.e. ensure all time units are in ms, or s when comparing them. Not to mention that it violates intuition when you could do subtract 2 Measures with different units and get a value that has a magnitude of 0, yet consider those units not equal.

Given these issues, I don't see how this change could be the right thing to do.

For your case, is it possible to trigger on the unit itself instead of the Measure? I don't know Compose well, but could you isolate the unit specified by the user and trigger recomposition on that?

@cvb941
Copy link
Author

cvb941 commented Aug 1, 2024

Well, the problem with the comparisons is, that you are comparing floating point values. They should be avoided as they are unreliable. Even in your example, 18 * inches == 1.5 * feet is false because 1.5 * feet is 17.999999999999996 inches.
At least converting a measurement to a different unit and comparing it to the old one indeed works.

For your case, is it possible to trigger on the unit itself instead of the Measure

Yes, that's basically my only option right now. I was trying to avoid it though, as it is easy to forget about this behaviour and prone to future bugs.

I understand you are not keen on changing the equality implementation. I wanted to open this discussion anyways to get your input. I will maybe fork this repository and make the changes for myself.

@pusolito
Copy link
Collaborator

pusolito commented Aug 2, 2024

Yeah, the equality issues between floating point values is unavoidable. I've considered providing a way to use Int or Long along with Double, but haven't landed on a good approach that keeps the lib simple. Would love to hear suggestions if you have any.

That said, the behavior of equality feels more intuitive in that it treats a Measure as some number with a scaling factor. This means 2 Measures are equal (precession issues aside) if the result of scaling them by their factors is also equal.

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

No branches or pull requests

2 participants