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

Add missing Option methods to Optioned #26

Open
4 of 7 tasks
CAD97 opened this issue May 2, 2018 · 5 comments
Open
4 of 7 tasks

Add missing Option methods to Optioned #26

CAD97 opened this issue May 2, 2018 · 5 comments

Comments

@CAD97
Copy link
Contributor

CAD97 commented May 2, 2018

  • as_ref(&self) -> Option<&T>
  • ok_or<E>(self, E) -> Result<T, E>
  • ok_or_else<E>(self, FnOnce() -> E) -> Result<T, E>
  • and<U>(self, Option<U>) -> Option<U>
  • and_then<U>(self, impl FnOnce(T) -> Option<U>) -> Option<U>
  • or(self, Option<T>) -> Option<T>
  • or_else(self, impl FnOnce() -> Option<T>) -> Option<T>

The ones that continually bite me are and(_then) and or(_else); I'm doing a lot of processing on space-conscious indices, and one of the operations that keeps repeating is "set if not set already". I'd like to write parent.child = parent.child.or(some(idx)) but for now I'm stuck using parent.child = some(parent.child.unwrap_or(idx)). (Actually, or_eq(&mut self, T) would be even cleaner -- parent.child.or_eq(idx) -- but a little too weird for my current tastes.)

@CAD97
Copy link
Contributor Author

CAD97 commented May 2, 2018

I'll probably send a PR in the morning with the or methods.

The and methods are a little harder because we'd probably want to support -> Optioned<U> and -> Option<U>.

Given as_slice(&self) and iter(&self).next(), is as_ref(&self)` still a useful method to have?

And finally, is ok_or(_else) useful to have, or even just worth it for completeness' sake of feature parity with std::option::Option?

@rnleach
Copy link
Contributor

rnleach commented Jun 3, 2018

@CAD97 pull request #29 that should take care of a few items on the list up top (and methods). I'm put in a pull request later for "ok" methods too.

@rnleach
Copy link
Contributor

rnleach commented Jun 3, 2018

@CAD97 pull request #34 will add the ok_or(_else) methods if it is merged.

I looked at adding as_ref() as well, but that is not so simple. The number types are copy anyway, so it doesn't really do anything there. Otherwise it requires that &T has Noned implemented as well. I suppose we could make a blanket implementation, but that's more than I want to work on just yet.

@CAD97
Copy link
Contributor Author

CAD97 commented Jun 3, 2018

as_ref would just go to Option<&T>: the null-pointer optimization is guaranteed there.

@rnleach
Copy link
Contributor

rnleach commented Jun 3, 2018

That could work. I've been using this a lot with arrays and Vec's of Optioned, and it can be painful jumping back and forth between Option and Optioned when dealing with iterators and functions like filter_map.

Plus From and Into are not ergonomic when dealing type inference. The compiler has a hard time figuring out what you want except in the simplest cases. So I've tried to avoid jumping between Optioned and Option. But it would be nice to jump back and forth sometimes for working with iterators.

On second thought, an as_ref could make an easy conversion too. Add that to making the as_option function public, it could really help. I'll have to experiment.

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

No branches or pull requests

2 participants