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 give method to Option<T> #1416

Closed
ghost opened this issue Dec 18, 2015 · 17 comments
Closed

Add give method to Option<T> #1416

ghost opened this issue Dec 18, 2015 · 17 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@ghost
Copy link

ghost commented Dec 18, 2015

Currently, Option<T> has the method take, which is implemented as replace(&mut self, None). To complement this method, I propose adding a give as well:

pub fn give(&mut self, val: T) -> Option<T> {
    ::std::mem::replace(self, Some(val))
}

#[test]
fn it_works() {
    let mut opt = None;
    assert_eq!(opt.give(5), None);
    assert_eq!(opt, Some(5));
    assert_eq!(opt.give(6), Some(5));
    assert_eq!(opt, Some(6));
}

This follows the same standard that HashMap, BTreeMap, and other collections use. Inserts a value into the option and returns the old value.

I feel like this is a common enough pattern to warrant a method from the standard library.

@ghost ghost changed the title Add insert method to Option<T> Add give method to Option<T> Dec 18, 2015
@steveklabnik
Copy link
Member

I haven't had a need for this, but I do like the symmetry 👍

@Stebalien
Copy link
Contributor

nit: personally, I'd call it offer.

@jonas-schievink
Copy link
Contributor

Rust giveth and Rust taketh away

@withoutboats
Copy link
Contributor

This seems to provide the same functionality as the methods requested in #1405 and #1278. While this proposal is symmetrical with take, I find the return value here really confusing (like difficult to reason about even with the code in front of me, which is not true of take) and don't know when it would be useful.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 21, 2015

I find this very confusing. HashMap and BtreeMap return the value that previously was in the slot. This api returns the value you just tried to put in. I find both solutions to be more readable in the following ways:

let x = val.take();
*val = Some(y);
if val.is_none() {
    *val = Some(x);
} else {
    // process val
}

@abonander
Copy link

I think put() makes more sense coupled with returning the previous value.

@withoutboats
Copy link
Contributor

So put would be something like this, right?

fn put(&mut self, val: T) -> &T {
    match *self {
        Some(ref item) => item,
        None => {
            self = Some(val);
            self.as_ref().unwrap()
        }
    }
}

@abonander
Copy link

@withoutboats More like

fn put(&mut self, val: T) -> Option<T> {
    mem::replace(self, Some(val))
}

For your suggestion, a more descriptive name like put_and_ref would probably be better.

@withoutboats
Copy link
Contributor

That would replace the value even if the value is Some; I think most of these requests have wanted the function to only insert in the None case.

@burdges
Copy link

burdges commented Dec 22, 2015

There is no reason to have a put that overwrites because you can always just overwrite it yourself by writing x = Some(y) or whatever.

Yes, give is meant to act like insert does on collections. I imagine the reasoning behind give is : If I've an Option<T> that I want to put into a struct but the location is occupied, then I want to handle it some other way, like by merging the T. It's maybe helpful if T is a Box or something.

I suppose a replace or swap function that always replaced the value and returns the contents sounds more intuitive, but maybe that's expensive if T is big. In principle, one could argue that if T were a Box, Arc<Mutex<S>>, etc. then T is small and replace is fast, and that big Ts should be handled another way.

@ghost
Copy link
Author

ghost commented Dec 22, 2015

Just to clarify, HashMap and BTreeMap do have this behaviour with insert. The main point of this method is to replicate that behaviour.

@withoutboats
Copy link
Contributor

The behavior @cybergeek94 suggested is the same as insert: it always replaces the value and returns the prior value.

@leoyvens
Copy link

I recently proposed something similar however I came to the conclusion that this would be a bad API because after being called it guarantees that the option will be Some, this encourages unwrapping and dosen't seem like a good use of Option after all.

@ghost
Copy link
Author

ghost commented Dec 26, 2015

Huh, I read this on the docs for insert a few weeks ago, and it must have changed.

In that case, I totally agree that a give that acts as replace(&mut opt, Some(val)) might be nice to have.

@BlacklightShining
Copy link

A give() that returns the value you pass if self.is_some() already would confuse me. I agree with calling that offer() and making give() unconditionally replace self. I dunno whether this is good API, though; I've never used take() or wanted give(), myself.

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Aug 19, 2016
@jaroslaw-weber
Copy link

unwraping option with result is nice but setting the value may be confusing when u dont know what is underneath the api. also writing Some(3) is not that inefficient

@Centril
Copy link
Contributor

Centril commented Oct 7, 2018

Tracked in rust-lang/rust#51998.

@Centril Centril closed this as completed Oct 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests