DanielKeep / rust-conv

Conversion traits for Rust
MIT License
48 stars 9 forks source link

Added MaybeFrom/MaybeInto traits #8

Closed panicbit closed 8 years ago

DanielKeep commented 8 years ago

Sorry for the delay in getting back to you.

I don't understand what this gets you that TryFrom/TryInto doesn't. The documentation also doesn't explain why it should be implemented or used.

panicbit commented 8 years ago

It should be used when a failed conversion doesn't signify an error per se. I felt like needing it for a macro, which creates types for each variant of an enum and allows converting between them if the variant and type match.

Originally I didn't want to add this to conv because of the overlap with TryInto / TryFrom, but someone in IRC also needed MaybeInto / MaybeFrom and encouraged me to make this PR. I'll try to get him to leave their thoughts here as well.

Xion commented 8 years ago

That someone would be me :)

My rationale is mostly about conversions defined as incomplete functions from one data type to another, where the only possible "error" is the value being out of domain. @panicbit has mentioned probably the best example: a conversion to an simple enum from a raw value:

enum Flag {
    Foo,
    Bar
}

impl<'i> MaybeFrom<&'i str> for Flag {
    fn maybe_from(input: &'i str) -> Flag {
        match input {
            "foo" => Some(Flag::Foo),
            "bar" => Some(Flag::Bar),
            _ => None,
    }
}

This can be, for example, parsing user-provided command line flags.

I suppose you could use TryFrom with GeneralError::Unrepresentable, but the you wouldn't be able to tell from the try_form method's interface alone that it is, in fact, the only variant of GeneralError that can occur.

DanielKeep commented 8 years ago

@panicbit The thing is that this essentially bifuricates the failable general conversions into two traits. Picking one or the other will keep you from using conversions defined using the other trait. Imagine if, in the stdlib, there was From/Into and OtherFrom/OtherInto that were equivalent, but incompatible. No one would ever know which one they're supposed to use. It'd be like AsRef vs Borrow, only worse.

I also don't feel this is compelling, given that on the user end of things, you can just add .ok() to throw away the error and convert from Result<T, _> to Option<T>.

@Xion This is the whole purpose of the Unrepresentable error type. Note that this is not a variant of one of the enums, it's a type all by itself.

Again, I don't find this argument compelling given that you can apparently already do this with conv. Is there some aspect to this I'm still missing?

DanielKeep commented 8 years ago

As a thought, it would be easy to add maybe_from and maybe_into as extension methods on ConvUtil. My concern with that is that they're rather trivial, and don't compose with anything else (I'm not big on adding a maybe variant for all of the Value*, Try*, and Approx* traits). That way, it's one less trait to have to deal with.

Can you show me a case where having maybe_into/maybe_from is a clear win over one of the existing methods plus .ok()?

panicbit commented 8 years ago

I agree that the overlap with TryFrom etc. is too big. If you think it doesn't add much to conv then feel free to close this PR.

Daniel Keep notifications@github.com schrieb am Sa., 9. Apr. 2016 06:25:

As a thought, it would be easy to add maybe_from and maybeinto as extension methods on ConvUtil. My concern with that is that they're rather trivial, and don't compose with anything else (I'm not big on adding a maybe variant for all of the Value, Try_, and Approx* traits). That way, it's one less trait to have to deal with.

Can you show me a case where having maybe_into/maybe_from is a clear win over one of the existing methods plus .ok()?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/DanielKeep/rust-conv/pull/8#issuecomment-207700534