Add equality operators for Period and ZoneInterval #1530
Conversation
|
|
||
| var failures = AllPublicTypes | ||
| .Where(t => typeof(IEquatable<>).MakeGenericType(t).IsAssignableFrom(t)) | ||
| .Where(t => t.GetMethod("op_Equality") is null) |
There was a problem hiding this comment.
Presumably reasonable to add a similar check here for op_Inequality as well?
There was a problem hiding this comment.
No need - the C# language won't let you overload == without overloading != too.
There was a problem hiding this comment.
Hurrah for common sense! (Another option would have been to generate synthetic operators given == and <, but this is good too.)
| .ToList(); | ||
| Assert.IsEmpty(failures); | ||
| } | ||
| } |
There was a problem hiding this comment.
At the risk of being somewhat pedantic, how about an additional test for IComparable types having all of {<, <=, >=, >} too?
There was a problem hiding this comment.
Hmm. I think we're likely to need a lot more exceptions for this, as it's quite reasonable to make things comparable without overloading those operators (e.g. string) but I'll try adding the test anyway - if we only need a few exceptions, it wouldn't hurt to have a reminder.
|
It turns out this was worth doing - I've added operators for YearMonth. PTAL. |
|
Just realized I need to add tests for the operators in YearMonth - that should be a matter of changing which helper method we call though. |
Additionally, add a check to ensure we don't miss this again, at least for anything implementing `IEquatable<T>` Fixes 1529.
Additionally, add a consistency check to avoid this problem happening again
Codecov Report
@@ Coverage Diff @@
## master #1530 +/- ##
=======================================
Coverage 99.23% 99.23%
=======================================
Files 153 153
Lines 6817 6829 +12
=======================================
+ Hits 6765 6777 +12
Misses 52 52
Continue to review full report at Codecov.
|
|
|
||
| var failures = AllPublicTypes | ||
| .Where(t => typeof(IEquatable<>).MakeGenericType(t).IsAssignableFrom(t)) | ||
| .Where(t => t.GetMethod("op_Equality") is null) |
There was a problem hiding this comment.
Hurrah for common sense! (Another option would have been to generate synthetic operators given == and <, but this is good too.)
Additionally, add a check to ensure we don't miss this again, at
least for anything implementing
IEquatable<T>Fixes #1529.