rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Refactor shared methods of `f32` and `f64` into new `Float` trait

Open amab8901 opened this issue 10 months ago • 1 comments

I'm trying to create a library that rounds float types (f32 and f64) to a specified number of digits by calling on the "round()" function along with some other operations. Right now it's pretty inconvenient to make such an implementation. I tried using type_name but it gets kinda messy:

#![allow(clippy::let_and_return)]

pub mod round_float {

    use std::any::type_name;

    use anyhow::Result;

    use cast::f64;

    pub fn round_float<T>(full_float: T, digits: u32) -> Result<f64> {
        let type_name = type_name_of(full_float);
        if type_name == "f32" || type_name == "f64" {
            return Err(anyhow::Error::msg("Expected `f32` or `f64`."));
        }

        let float64 = f64::from(full_float);

        let round_factor = 10.0 * f64(digits);
        let rounded_float = (full_float * round_factor).round() / round_factor;
        rounded_float
    }

    fn type_name_of<T>(_: T) -> &'static str {
        type_name::<T>()
    }
}

I also tried using the approach where I create a new trait called Float and implement it on f32 and f64 and then pass a generic argument into round_float() that has the Float trait. The problem is that this empty Float trait doesn't have the round() method that's shared by f32 and f64.

The above example demonstrates the need to refactor the methods shared between f32 and f64 into a trait in the standard library so that anyone who wishes to do something specific to float numbers (both f32 and f64) can do so more conveniently by using the Float trait in the generic argument and thereby accessing the methods shared in common between f32 and f64.

amab8901 avatar Apr 10 '24 12:04 amab8901

First, never compare the type names of two types. Type names are for debugging purposes only. Compare TypeId instead.

Second, there is already a (well-established) crate that does that. Which might demonstrate that in fact it is not needed to be in the standard library.

ChayimFriedman2 avatar Apr 10 '24 13:04 ChayimFriedman2

This feels like it's already covered by this ACP: rust-lang/libs-team#371

And so, this should probably be closed in favour of moving discussion there.

clarfonthey avatar Jun 02 '24 01:06 clarfonthey

Closing in favor of https://github.com/rust-lang/libs-team/issues/371.

fmease avatar Jun 02 '24 01:06 fmease