Should we add `map_none` method for `Option` type?

libs
1

I noticed that we have the map_err method for Result type. It returns Err(op(e)) if the receiver is Err. The Result enum also has or_else method, which just returns op(e) if the receiver is Err.

Similar to Result enum, Option has or_else method too, which returns the closure result f() if the receiver is None. But I do not find any method that returns Some(f()) if the receiver is None in Option enum.

Should we add a method (named like map_none) to support it?

Code like:

pub fn map_none(self, f: impl FnOnce() -> T) -> Option<T> {
    match self {
        Some(_) => self,
        None => f().into(),
    }
}
1 Like
2

Result::map_err takes E1 -> E2 to perform Result<T, E1> -> Result<T, E2>. To be symmetric the Option::map_none should take () -> () to perform Option<T> -> Option<T> which doesn't make sense to me.

This function never returns None, actually opt.map_none(f) is identical with the Some(opt.unwrap_or_else(f)). What would be the use case of this implicit Some wrapping?

5 Likes
3

I think map_none (or another name) should map None to Some for convenience.

Use case:

struct HttpResponse<'a> {
    headers: Option<HashMap<&'a str, &'a str>>,
}

impl<'a> Default for HttpResponse<'a> {
    fn default() -> Self {
        HttpResponse {
            headers: None,
        }
    }
}

impl<'a> HttpResponse<'a> {
    pub fn new(headers: Option<HashMap<&'a str, &'a str>>) -> Self {
        let mut response = HttpResponse::default();
        // or_else:
        response.headers = headers.or_else(|| HashMap::new().tap_mut(|h| h.insert("xx", "zz")).into());
        
        // unwrap_or_else:
        response.headers = headers.unwrap_or_else(|| HashMap::new().tap_mut(|h| h.insert("xx", "zz"))).into();
        
        // map_none:
        response.headers = headers.map_none(|| HashMap::new().tap_mut(|h| h.insert("xx", "zz")));
    }
}
4

Since it always returns Some(..), your foo.map_none(bar) is basically the same as Some(foo.unwrap_or_else(bar)); the latter is not significantly longer and does a better job at highlighting the fact that it's always Some, so I don't feel like there's API missing here.

13 Likes
5

So to apply my suggestion above to this code example, I would re-write that as

use std::collections::HashMap;

struct HttpResponse<'a> {
    headers: Option<HashMap<&'a str, &'a str>>,
}

impl<'a> Default for HttpResponse<'a> {
    fn default() -> Self {
        HttpResponse { headers: None }
    }
}

impl<'a> HttpResponse<'a> {
    pub fn new(headers: Option<HashMap<&'a str, &'a str>>) -> Self {
        HttpResponse {
            headers: Some(headers.unwrap_or_else(|| HashMap::from_iter([("xx", "zz")]))),
        }
    }
}
2 Likes
7

Since it always returns Some(..) , your foo.map_none(bar) is basically the same as Some(foo.unwrap_or_else(bar)) ; the latter is not significantly longer and does a better job at highlighting the fact that it's always Some , so I don't feel like there's API missing here.

According to this, map_err shouldn't exist either.

8

headers: Some(headers.unwrap_or_else(|| HashMap::from_iter([("xx", "zz")])))

I believe that the level of parentheses is too deep, which will make us difficult to change the code.

If you add some tuples to the HashMap's value, which probably makes the code more difficult to read and modify.

9

Well, Result's map_err does not always return Ok, or always return Err. So I guess I don't quite see the connection.

Viewing Option<T> as something that's almost the same as Result<T, ()>, I'd come to the same conclusion as @hyeonu above, that a an Option::map_none should, if it exists, call a FnOnce() -> () callback, or possibly FnOnce(()) -> (), which doesn't really do anything of transforming the Option value though. Just executes some potential side-effects in the None case; which could probably be more clearly achieved with an if foo.is_none() { /* do something *}; /* keep using `foo` */

I'll admit, it's an impressive amount of closing parentheses in the end.

I feel like rustfmt does an okay job to keep it readable. E.g.

impl<'a> HttpResponse<'a> {
    pub fn new(headers: Option<HashMap<&'a str, &'a str>>) -> Self {
        HttpResponse {
            headers: Some(headers.unwrap_or_else(|| {
                HashMap::from_iter([
                    ("xx", "zz"),
                    ("yy", "aa"),
                    ("zz", "bb"),
                    ("aa", "f"),
                    ("foo", "bar"),
                ])
            })),
        }
    }
}

Alternatively, some indentation and parentheses can be saved by using a local variable, e.g.

impl<'a> HttpResponse<'a> {
    pub fn new(headers: Option<HashMap<&'a str, &'a str>>) -> Self {
        let headers = headers.unwrap_or_else(|| {
            HashMap::from_iter([
                ("xx", "zz"),
                ("yy", "aa"),
                ("zz", "bb"),
                ("aa", "f"),
                ("foo", "bar"),
            ])
        });
        HttpResponse {
            headers: Some(headers),
        }
    }
}

well, actually that saves less indentation than I thought it would... anyways, the use of from_iter was just an unrelated suggestion that you can follow if you want to, but don't have to.

4 Likes
10

You are right, but could you also consider ergonomics?

Whether it is adding Some() to the front or extracting the contents into a variable, it seems a bit anti-human...

Adding Some() you should press Shift twice and go across all middle code, then code ) at the end. Or, your hand needs to leave the keyboard and use the mouse to move the cursor long distances to the position you want. It's not convenient.

I know that some people may be accustomed to using vim and use some shortcut keys to solve this problem, but this is not suitable for me.

Extracting the contents into a variable is more complex. Repeated Ctrl-C and Ctrl-V, but also define variables, and the hand needs to leave keyboard to mouse repeatedly too. It's inconvenient.

In order to reduce the frequency of the brain asynchronously doing context switching (the brain's cache is less than the CPU's first-level cache), I believe that we should provide more user-friendly APIs for developers to use as much as possible.

Result::map_err takes E1 -> E2 to perform Result<T, E1> -> Result<T, E2> . To be symmetric the Option::map_none should take () -> () to perform Option<T> -> Option<T> which doesn't make sense to me.

According to @hyeonu , I think that we probably need another name to do this.

What's a better name for map_none, if a developer really thinks he needs that?

11

Your map_none returns only Some. It never returns None.

In other words, the Option is unnecessary. Wrapping the data with Option gives no additional information if None is not really an option... (pun intended).

So unwrap_or_else is what you really need here. That's why there is no map_none.

7 Likes
12

Although map_none returns only Some, this is a partial situation for the field.

The field can be changed to None in other functions. But in this part should always return Some.

13

or_some plus some variation for the closure version,

  • or_else_some
  • or_some_else
  • or_some_with

Ala get_or_insert/get_or_insert_with and or(Some(...))/or_else(|| Some(...)).

(Though I'm also unconvinced they're needed, and feel expanding APIs too eagerly can be a disservice to the developers.)

1 Like
14

I tend to do opt.or_else(|| Some(f())), which I think is clean enough. There's also opt.or(Some(foo)).

2 Likes
15

Don't optimize for writability, optimize for readability.

6 Likes
16

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.