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.

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

Feature suggestion to apply into() on non-optional and optional values #4230

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sveamarcus
Copy link

@sveamarcus sveamarcus commented Oct 8, 2023

This is a minimal feature request/suggestion to lessen the boilerplate of dealing with optionals. Consider this convoluted example

fn foo() -> Option<bool> {
  call_with_optional_result()?.into()
}

instead of requiring the manual constructor Option::Some(call_with_optional_result()?), which to me loses some clarity of expression.

It broadens the use of .into() to either wrap a non-optional value into its expected optional result or, in the case where there is a clear type conversion such as an upcast (into) or downcast (try_into) between optional types, those cases can be considered as well.

Maybe the scope of into() is inappropriate for the suggested behavior here. In some languages, perhaps the more natural applications of dealing with optional type conversion is through (functor) maps.

N.b. that a type conversion between Option<A> to Option<B> where there exists TryInto<A, B>, is possible with an .into() as opposed to a .try_into() as well, but semantically it may cause more harm than good. Consider this in contrast to what was committed in the PR

impl TryIntoOptionFromOptionImpl<T, S, +TryInto<T, S>> of Into<Option<T>, Option<S>> {
    #[inline]
    fn into(self: Option<T>) -> Option<S> {
        match self {
            Option::Some(t) => t.try_into(),
            Option::None => Option::None,
        }
    }
}

This change is Reviewable

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shorthand is only adding confusion for this case.
specifically Option to Option mapping should be implemented using .map when closures are added.

If you want to have these sort of features - named impl can provide you this sort of feature for you lib only.
just have this impl in a lib - and where you want to use this - add a use path/to/impl;in the file.

Reviewable status: 0 of 1 files reviewed, all discussions resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants