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

AutoRouterImproved extendability #7364

Open
element-code opened this issue Mar 20, 2023 · 4 comments
Open

AutoRouterImproved extendability #7364

element-code opened this issue Mar 20, 2023 · 4 comments
Labels
enhancement PRs that improve existing functionalities

Comments

@element-code
Copy link
Contributor

In the current state, the AutoRouterImproved is not thought to be replaced by an App-Specific service and to be extended on.

kenjis: Because it is better to implement the interface, not extending the class.

In my case, I need to change the controller & method name which is already implemented in the router.
Re-Implementing the AutoRouterImproved to do that seems kinda ridiculous.

Two approaches come to my mind:

  1. The specific solution: Use the "parent" routers methods for finding the method and controller name, this prevents duplicate code and allows for a little app-specific behavior.
  2. The generic solution: Make the auto routers replaceable by app-specific services
    1. Optional: make both extendable, so we don't have to write ridiculous amounts of code for small changes.
@kenjis
Copy link
Member

kenjis commented Mar 20, 2023

I don't understand what you say.

AutoRouterImproved has no parent class.

I need to change the controller & method name which is already implemented in the router.

You can extend Router now.

If you change the controller & method name in AutoRouterImproved, would it violate the LSP?
How do you change the controller & method name?

@element-code
Copy link
Contributor Author

element-code commented Mar 21, 2023

AutoRouterImproved has no parent class.

Both auto routers are initialized by the Router, which provides public methods controllerName, methodName
I'm pro using those both methods in both the AutoRouter and the AutoRouterImproved instead of passing the translateURIDashes configuration to the auto routers.

You can extend Router now.

Yes, but the changes to controllerName, methodName are ignored by the "new" auto routers

How do you change the controller & method name?

Similar to translate-uri-dashes i am translating them to camel/pascal case instead of snake case

If you change the controller & method name in AutoRouterImproved, would it violate the LSP?

Using the routers public methods controllerName, methodName would assure that all routing is based on the same name translation logic

@kenjis
Copy link
Member

kenjis commented Mar 21, 2023

In my opinion, controllerName() and methodName() in Router should be just getters.
They should just return the properties, should not have any logic.

@kenjis
Copy link
Member

kenjis commented Mar 22, 2023

There are now three routers or route handlers (defined routes, auto routes improved, auto routes legacy).
We have a plan to make them stackable. Also devs will be able to add their own routers and remove unneeded one.

@kenjis kenjis added the enhancement PRs that improve existing functionalities label Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PRs that improve existing functionalities
Projects
None yet
Development

No branches or pull requests

2 participants