Skip to content

get_or_default #33

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

get_or_default #33

wants to merge 2 commits into from

Conversation

arcnmx
Copy link

@arcnmx arcnmx commented Jan 11, 2016

Also exposes Implements publicly so that it can be used as a generic bound in external crates.

@reem
Copy link
Owner

reem commented Jan 12, 2016

I'd like to hear what the use case for Implements in another crate is, I originally meant to keep it internal. I'm not sure it's really worth having a method for something that's already a simple one-liner out of composable pieces, do you have some specific case that this makes prettier? I would be more open to an or_default method on the Entry object directly, since that seems more fine-grained.

@arcnmx
Copy link
Author

arcnmx commented Jan 12, 2016

The thing about a private Implements is that it makes it impossible to write extension traits or generic methods that call the AnyMap methods over a generic TypeMap<A>, since you can't express the type bounds on a generic Key::Value. So, for example, a method much like get_or_default is super heavily used in my codebase, and it simply can't be abstracted away: I have to expose a mutable reference to the TypeMap field directly and then maybe use a macro if I wanted to shorten it.

@reem
Copy link
Owner

reem commented Jan 12, 2016

So if Implements was public, you would be fine without get_or_default?

@arcnmx
Copy link
Author

arcnmx commented Jan 13, 2016

Sure, that seems fine to me. From my reading of RFC 136, it sounds like a bug that the compiler allows it to be private at all..?

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.

2 participants