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

Make EventBus(EventBusBuilder) constructor public. #281

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trashkalmar
Copy link

There are scenarios when it is better to use EventBus as base class. In this case it is impossible to construct custom class which is configured by EventBusBuilder builder.
This PR makes the EventBus(EventBusBuilder) constructor public to support latter case.

@zxw1962
Copy link

zxw1962 commented Apr 14, 2016

But this will break the build pattern, if so, user can just new EventBus instance directly, which will make EventBusBuilder.build method less important.

IMO, If there are many choices, there's no choices. What do you say?

@trashkalmar
Copy link
Author

Why? User can continue to use pattern and create EventBus instance with builder. However, at this time I'm unable to use EventBus as base class and create descendants configured with the builder.

@william-ferguson-au
Copy link
Contributor

william-ferguson-au commented May 12, 2016

What's the use case for subclassing EventBus?
I would instead argue for making the class final and explicitly disallow subclassing.

@trashkalmar
Copy link
Author

Use case is very simple. I'd like to perform some action while registering event handler, i.e. override register/unregister methods in EventBus. Also I'd like to customize EventBus`s behavior with Builder (disable event inheritance in my case).
However, at this time I have to create wrapper and pass register/unregister/post/postSticky calls to wrapped EventBus instance configured with Builder.

@trashkalmar
Copy link
Author

@greenrobot so, what do you think?

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.

3 participants