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.

Sign up for GitHub

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

Jump to bottom

WellKnownTypes.Duration does not implement IComparable #7628

Closed
aub22angel opened this issue Jun 15, 2020 · 2 comments · Fixed by #10441
Closed

WellKnownTypes.Duration does not implement IComparable #7628

aub22angel opened this issue Jun 15, 2020 · 2 comments · Fixed by #10441
Assignees
@jskeet
Labels
c# help wanted

Comments

@aub22angel
Copy link

aub22angel commented Jun 15, 2020 •
edited
Loading

What language does this apply to?
If it's about generated code change, what programming language? C#

Describe the problem you are trying to solve.
I am trying to perform comparisons on the Duration type for data validation. I get a runtime exception when I run this code:

ComparativeValidation.LessThanOrEqual("x", Duration.FromTimeSpan(_timespanOf10), 
Duration.FromTimeSpan(_timespanOf20), Comparer<Duration>.Default); // At least one object must implement IComparable

Describe the solution you'd like
Can the Duration type implement the IComparable interface?

Describe alternatives you've considered
For our immediate need, we are pursuing a manual comparison method for the seconds and nano properties.

Additional context
This request is similar to the Timestamp request.
@haberman haberman added the c# label Aug 13, 2020
@deannagarcia
Copy link
Member

Sorry for the late response. Unfortunately we don't currently have the bandwidth to support this but it would be great if someone sent us a PR for it!

@jskeet jskeet self-assigned this Aug 23, 2022
@jskeet
Copy link
Contributor

jskeet commented Aug 23, 2022

(I'm implementing this now.)

jskeet added a commit to jskeet/protobuf that referenced this issue Aug 23, 2022
@jskeet
Implement IComparable<Duration> for the Duration type (C#)
8fd097b 
Note that unlike Timestamp, this does *not* also overload comparison
operators. Adding a `==` overload now would be a breaking change (as
it would change the meaning of existing code from a reference
comparison to a value comparison), and implementing `<`, `<=`, `>=`
and `>` without implementing `==` would be odd.

Implementing `IComparable<T>` makes sorting much easier, however.

Fixes protocolbuffers#7628
@jskeet jskeet mentioned this issue Aug 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jskeet jskeet

Labels
c# help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement IComparable<Duration> for the Duration type (C#) jskeet/protobuf
4 participants
@haberman @jskeet @aub22angel @deannagarcia