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 Features to OptionalCell #1034

Merged
merged 3 commits into from Jun 27, 2018

Conversation

Projects
None yet
3 participants
@bradjc
Copy link
Contributor

bradjc commented Jun 26, 2018

Pull Request Overview

This pull request adds some normal Option functions to OptionalCell.

Actual uses coming in due time.

Testing Strategy

This pull request was tested by compiling.

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

@bradjc bradjc force-pushed the optionalcell-enhance branch from 3cc8485 to 3c08db3 Jun 26, 2018

/// Call a closure on the value if the value exists.
pub fn map<F, R>(&self, closure: F) -> Option<R>
where
F: FnOnce(&mut T) -> R,
{
self.value.get().map(|mut val| closure(&mut val))
}

/// Call a closure on the value if the value exists, or return value

This comment has been minimized.

@ppannuto

ppannuto Jun 26, 2018

Member

Are there two different values in this sentence? One is the value inside the cell if it's full, the other is the value passed to the function, right? Can we call one the cell contents or something? I had trouble reading this

@bradjc bradjc force-pushed the optionalcell-enhance branch from 2a8ce92 to b3b55e2 Jun 27, 2018

@niklasad1
Copy link
Member

niklasad1 left a comment

Looks good to me, any reason why we don´t provide at map_or_else? map_or is eagerly evaluated and is not suitable if default is something that needs to be evaluated e.g. a function call!

@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Jun 27, 2018

No reason in particular, I just implement them as I use them.

@ppannuto ppannuto force-pushed the optionalcell-enhance branch from 47f6840 to df3f9e7 Jun 27, 2018

@ppannuto
Copy link
Member

ppannuto left a comment

Hah. I dismissed my own review by updating that doc comment. Sometimes software is silly.

@ppannuto ppannuto merged commit 8bc5ce8 into master Jun 27, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ppannuto ppannuto deleted the optionalcell-enhance branch Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment