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 option::get_zero #4340

Closed
wants to merge 1 commit into from
Closed

Conversation

catamorphism
Copy link
Contributor

r? @brson - as per #3797

@brson
Copy link
Contributor

brson commented Jan 4, 2013

r+

@catamorphism
Copy link
Contributor Author

Merged, 1f1e7e9

@kud1ing
Copy link

kud1ing commented Jan 4, 2013

get_zero is in line with get_default. But to me it sounds a bit like "give zero".

How about get_or_zero and get_or_default?

@nikomatsakis
Copy link
Contributor

I personally'd prefer @kud1ing's suggestions. The name get_default() always throws me when I read it.

@erickt
Copy link
Contributor

erickt commented Jan 4, 2013

+1 to @kud1ing's naming scheme.

@kud1ing
Copy link

kud1ing commented Jan 4, 2013

I often think, that variable/function names should not be less descriptive than their short documentation string. And in this case it is "Returns the contained value or zero".

That's why i'd even prefer value_or_zero() and value_or_default() but maybe that would be considered too long?

@nikomatsakis
Copy link
Contributor

I think they should use the same prefix as get(). And I prefer opt.get() to opt.value(). It's shorter but also using a verb conveys the fallibility to me. value() sounds like it's just a field getter.

@catamorphism
Copy link
Contributor Author

Seems like the masses have spoken in favor of get_or_default and get_or_zero, so I checked in 89acd1f

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

Successfully merging this pull request may close these issues.

5 participants