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

open fun in open class Router #146

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

Conversation

Starksoft
Copy link

@Starksoft Starksoft commented Feb 17, 2021

I'm using custom Router implementation and use internal logger. Router is open class, but have final methods, I need them to be open (custom logger & custom RouterImpl)

@AlexeyKorshun
Copy link

It is not good idea. But it is a good point. @terrakok maybe we should create an interface for router?

@Starksoft
Copy link
Author

Starksoft commented Mar 2, 2021

It is not good idea. But it is a good point. @terrakok maybe we should create an interface for router?

I really don't understand, why it is "bad" idea.

If I want to extend the functionality of this Router all methods must be non-final.
In this case, the developer takes responsibility for the incorrect behavior of the library, after his intervention.

In my case I incapsulate authorization checks in RouterWrapper, and check if screen can be opened without auth or open login screen otherwise

@terrakok
Copy link
Owner

terrakok commented Mar 2, 2021

For your case more suitable approach is wrap router with your logic which will delegate calls to inner router.
For easy swap Cicerone router to your Business router with DI it is reasonable to extract interface from router

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