Skip to content

Tracking issue for Ord::{min, max} #25663

Closed
@kornelski

Description

@kornelski
Contributor

It's seems inconsistent that floating-point types have a min method, but integer types don't.

And there's a standalone min() function that works with integer types, but it's not defined for floating-point types.

I don't have a strong preference for either style, but I'd prefer that at least one way of computing min/max was available for both floating point and integer types alike.

Activity

added
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on May 21, 2015
Aatch

Aatch commented on May 21, 2015

@Aatch
Contributor

@aturon thoughts? Seems like adding an inherent method to integer types is the easiest way forward here.

Note that there are partial_min and partial_max functions for types that only implement PartialOrd.

aturon

aturon commented on May 21, 2015

@aturon
Member

+1 for an inherent method. I think this is just an oversight.

alexcrichton

alexcrichton commented on May 26, 2015

@alexcrichton
Member

I believe @bluss commented to this effect already, but the idea here is that there is "one true min method" at cmp::min, but the floating point types use a slightly different algorithm (e.g. different guarantees about order and whatnot), hence they have their own methods.

I wouldn't be totally opposed to adding an inherent method, but we'd want to clearly document that it's just an alias to cmp::min

nagisa

nagisa commented on Jun 17, 2015

@nagisa
Member

-1 for an inherent (or any other, really) method. Float’s min and max are just bindings to libm f{min.max}{f,}, while cmp::partial_{min,max} have their own, sane if you will, semantics. Adding inherent methods for integral types will make many-ways-to-do-the-same problem worse.

frewsxcv

frewsxcv commented on Dec 14, 2016

@frewsxcv
Member

Ignoring the "many-ways-to-do-the-same problem" for a second, couldn't min and max just be generic 'provided methods' on the Ord trait?

kornelski

kornelski commented on Dec 14, 2016

@kornelski
ContributorAuthor

For me this is a practical problem, because changing type needlessly changes syntax.

let a = 1.0;
let b = 2.0;
let c = a.max(b); // c = std::cmp::max(a,b)

I can't change a = 1; b = 2 without rewriting the third line.

I often write performance-sensitive graphics code, which I prototype on floats for ease of writing, and then where appropriate change to use integers for speed/memory efficiency. 90% of calculations just work, and max is one of the warts that doesn't.

Mark-Simulacrum

Mark-Simulacrum commented on May 18, 2017

@Mark-Simulacrum
Member

@rust-lang/libs: Looks like this needs some thought; I've nominated. Possibly interesting for libz blitz discussion, too, since it relates to number types in wider library set; for example serde_json::Number doesn't implement min/max inherently, but should it?

I think that the things that have been brought up are:

aturon

aturon commented on May 23, 2017

@aturon
Member

We discussed this in triage today, and the libs team is in favor of adding these methods to Ord

Mark-Simulacrum

Mark-Simulacrum commented on May 23, 2017

@Mark-Simulacrum
Member

[De-nominating]

25 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    B-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCT-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @alexcrichton@kornelski@Aatch@frewsxcv@nagisa

        Issue actions

          Tracking issue for Ord::{min, max} · Issue #25663 · rust-lang/rust