-
Notifications
You must be signed in to change notification settings - Fork 396
✨Add Equals() extension methods for untyped IQuantity #1598
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
base: master
Are you sure you want to change the base?
Conversation
- Remove `Equals` overloads taking untyped `IQuantity other` from `LinearQuantityExtensions`, `AffineQuantityExtensions`, `LogarithmicQuantityExtensions` - Add `QuantityExtensions.Equals()` with only untyped `IQuantity` parameters - Move/copy relevant tests over to `QuantityExtensionsTests`
@lipchev Here is an attempt at having untyped There is a TODO to fix the implementation and delegate to the linear/affine/logarithmic equals, but due to them taking generic type params more work is needed and I ran out of time. If you like this direction, please help out making it work. I'll probably be less active the next few days again, work and life is resuming tomorrow. |
/// <param name="other"></param> | ||
/// <param name="tolerance"></param> | ||
/// <returns></returns> | ||
public static bool Equals(this IQuantity quantity, IQuantity? other, IQuantity tolerance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to double-check which cases are hitting this, but even if it works as expected - I really don't see how anyone could ever use this.. If you don't know what type of quantity you're working with, how can you select the appropriate tolerance?
I was hoping you'd pick option 3, instead you picked option -1 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK you have me convinced.
There are consumers of the untyped IQuantity
, so it just seemed to me like a natural thing to support equality checks for, since we do offer this interface. Mostly for completeness sake.
But I totally get your point that passing an untyped tolerance seems very fringe. A custom equality comparer to reuse for different quantities is the best I can come up with, but even then I'm sure a generic typed to either linear/affine/logarithmic would be better.
With the fraction PR and accurate equality, I suppose tolerance equality becomes less of a thing.
I do think there are some improvements to the type parameters here, to have fewer of them, but it still can be reduced yet more. Let me give it another go and update this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think there are some improvements to the type parameters here, to have fewer of them, but it still can be reduced yet more. Let me give it another go and update this PR.
Yes I know the default operator is a bit weird- I was surprised you didn't ask about it before:
public static bool Equals<TQuantity, TOther, TTolerance>(this TQuantity quantity, TOther? other, TTolerance tolerance)
where TQuantity : ILinearQuantity<TQuantity>
where TOther : IQuantityOfType<TQuantity>
where TTolerance : IQuantityOfType<TQuantity>
Technically, here we could probably get away without a separate TTolerance
(re-using TQuantity
), but while prototyping it, I was intentionally trying to keep everything to the lowest constraint-level (having just finished a complete implementation of the INumber
interface), even if the actual use cases that I was able to imagine for it are probably YAGNI- here's something that would be cool to try:
public class Tolerance<TQuantity> : IQuantityOfType<TQuantity>
where TQuantity : ILinearQuantity<TQuantity>
There shouldn't be much to implement here, and if one was, you know, obsessed, they could have domain types like ScrewTolerance
, which, if defined as a constant/configuration property, would probably work better as class (requiring less copying overhead).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I just got the release for resharper today- and dammit, it's not the right version- we got version 2025.1.5
which, if I understand their versioning schema, means that our fix is not in it.. I'm really surprised that they would delay its release for so long, and even more- that nobody else has complained about it. 🙄
Make it easier to work with untyped
IQuantity
, and favor typed extensions methods when working with concrete types.Equals
overloads taking untypedIQuantity other
fromLinearQuantityExtensions
,AffineQuantityExtensions
,LogarithmicQuantityExtensions
QuantityExtensions.Equals()
with only untypedIQuantity
parametersQuantityExtensionsTests
TODO
10 test cases currently break due to having a generic implementation, need to delegate to the implementations for linear, affine and logarithmic.