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

Allow services to be scoped. #133

Merged
merged 2 commits into from
Mar 11, 2015
Merged

Allow services to be scoped. #133

merged 2 commits into from
Mar 11, 2015

Conversation

rjrjr
Copy link
Collaborator

@rjrjr rjrjr commented Mar 11, 2015

BundleServiceRunner uses this to minimize the keys it writes
to the bundle, thus keeping it safe for apps to use crazy
transient values like task-id in their activity scope names.

BundleServiceRunner uses this to minimize the keys it writes
to the bundle, thus keeping it safe for apps to use crazy
transient values like task-id in their activity scope names.
* Makes this service available via the new scope's {@link MortarScope#findService}
* method. If the service implements {@link Scoped} it will be registered with
* the new scope.
*/
public Builder withService(String serviceName, Object service) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the use case for a service that doesn't want to implement Scoped?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't a fan of the hidden behavior in register() where the Scoped was also a Bundle... compile time APIs are much clearer and safe than javadoc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: use case, neither dagger service needs it. I suspect most won't.

I can add a withScopedService(String, Scoped)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed offline, but for the record: now tend to agree with PY, a second method with a Scoped param is probably better than the instanceof check. The method w/ Object could do an instanceof check to guard against accidental usage of the wrong method.

@pyricau
Copy link
Member

pyricau commented Mar 11, 2015

LGTM

rjrjr added a commit that referenced this pull request Mar 11, 2015
@rjrjr rjrjr merged commit 6e7920a into master Mar 11, 2015
@rjrjr rjrjr deleted the ray/dont-fear-the-bundle branch April 28, 2015 01:25
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.

None yet

3 participants