Skip to content

Make EventBus(EventBusBuilder) constructor public.#281

Open
trashkalmar wants to merge 1 commit intogreenrobot:masterfrom
trashkalmar:make-builder-constructor-public
Open

Make EventBus(EventBusBuilder) constructor public.#281
trashkalmar wants to merge 1 commit intogreenrobot:masterfrom
trashkalmar:make-builder-constructor-public

Conversation

@trashkalmar
Copy link
Copy Markdown

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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