optional icon indicating copy to clipboard operation
optional copied to clipboard

Add missing Option methods to Optioned

Open CAD97 opened this issue 6 years ago • 5 comments

  • [ ] as_ref(&self) -> Option<&T>
  • [ ] ok_or<E>(self, E) -> Result<T, E>
  • [ ] ok_or_else<E>(self, FnOnce() -> E) -> Result<T, E>
  • [x] and<U>(self, Option<U>) -> Option<U>
  • [x] and_then<U>(self, impl FnOnce(T) -> Option<U>) -> Option<U>
  • [x] or(self, Option<T>) -> Option<T>
  • [x] 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 avatar May 02 '18 05:05 CAD97

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?

CAD97 avatar May 02 '18 05:05 CAD97

@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 avatar Jun 03 '18 00:06 rnleach

@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.

rnleach avatar Jun 03 '18 02:06 rnleach

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

CAD97 avatar Jun 03 '18 02:06 CAD97

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.

rnleach avatar Jun 03 '18 04:06 rnleach