Add clamp function

[Xaeroxe]

This PR adds a "clamp" function to num. Oftentimes it is desirable to restrict a value to between a certain minimum and maximum, the clamp function provides a generic way to do this.

[Xaeroxe]

The build fails before it even starts compiling num. The build server is likely misconfigured.

[cuviper]

The 1.0 failure is #257 -- nothing you can can fix here.

I'll give a review here in a little bit.

[clarfon]

Why does this require Copy? It doesn't seem necessary.

[cuviper]

I agree with @clarcharr that this shouldn't need Copy.

[cuviper]

I think min <= max is fine, if we're going to assert anything.

[cuviper]

This formatting is non-standard -- please run it through rustfmt.

I think it would be slightly preferable to return input when it is equal to the boundaries. This only matters for cases similar to stable sorting, where there may be internal data that's not part of the ordering. So just compare input < min and input > max.

[cuviper]

A few nits -- sorry I failed to batch the GitHub review.

Also, we've moved away from any real functionality in num itself. I guess this should go into num-traits.

It needs at least some basic tests too.

[cuviper]

A few nits -- sorry I failed to batch the GitHub review.

Also, we've moved away from any real functionality in num itself. I guess this should go into num-traits.

It needs at least some basic tests too.

[Xaeroxe]

Yeah good point. Copy made sense at the time but it's not really needed.

Requested changes have been made.

[clarfon]

Another thing is that it'd be best to match on Ord::cmp instead of doing ifs.

[cuviper]

Another thing is that it'd be best to match on Ord::cmp instead of doing ifs.

Why? Then it will require Ord too.

[Xaeroxe]

The alternative is partial_cmp but I don't understand what benefit we gain from using that.

[Xaeroxe]

use std::cmp::Ordering;
match input.partial_cmp(&min) {
    Some(Ordering::Less) => min,
    _ => 
        match input.partial_cmp(&max) {
            Some(Ordering::Greater) => max,
            _ => input
        },
}

This is my best attempt at implementing it using partial_cmp. I don't really like it.

[cuviper]

@clarcharr See rust-lang/rust#39538, which found in that case that binary comparisons perform better than a ternary cmp. Unless you can justify your suggestion further, I'm inclined to merge this as-is.

[Xaeroxe]

The suggestion would make sense if it were possible to implement it using just one match statement, but because it is being compared to two values, not one, the suggestion doesn't really work.

[cuviper]

Merged - thanks!