Skip to content
This repository has been archived by the owner on Feb 10, 2023. It is now read-only.

One controller, one action. #26

Open
emlynwest opened this issue Apr 18, 2017 · 11 comments
Open

One controller, one action. #26

emlynwest opened this issue Apr 18, 2017 · 11 comments
Milestone

Comments

@emlynwest
Copy link
Contributor

It will make controllers more uniform, simpler, no issues with action routing or checking, before() can be dropped, __invoke() can be used, etc. Abstract base classes or Traits can be used for shared logic.

@emlynwest
Copy link
Contributor Author

Personally I don't agree with this, I think it may end up with lots of unnecessary controllers. I think that multiple actions should be supported and it should be up to the developer to decide how to structure their application.

@sagikazarmark
Copy link

Agreed. A common pattern is to provide actions as callables. Even it happens under the hood, it's a generic way to support multiple actions/controller, one controller with __invoke or just a callable.

I really like the FastRoute way.

@WanWizard
Copy link

It is one of the things I have against the current trend of having to code and define everything. The strength of Fuel compared to other frameworks is its simplicity, and I think that should remain our first design principle.

It is definitely true that it will lead to a lot more files, as you need a controller class for every action. But it makes automatic URI to Controller mapping a lot easier (it will be one-on-one), you don't need all the logic to deal with multiple actions, no router() method, no action checking in your before() if you need prepping for one action but not for others, you can have standardized http-method methods, it is easier and better to cache, you can overload controllers per action using DI, etc.

@sagikazarmark
Copy link

One thing is what we recommend and what we mark as best practice, another thing is what happens under the hood.

URI to Controller mapping

This suggests that you want to couple the routing component to some arbitrary Controller structure. I think the router should not know where it routes to. If you mean this would make registering routes in the router easier, then I totally agree. But that's not a reason why we could not support multiple actions/controller (not from router nor from controller perspective).

All the other arguments are related to how the controller works internally, and it should be up to the developer IMHO.

@emlynwest
Copy link
Contributor Author

I'm with @sagikazarmark on this one, I've no issue with developers using one or multiple actions per controller but I don't think either method should be forced on the developer. The logic for the routing need not be super complicated, I think even with the extra before/after methods the framework code can still be simple and clean.

@WanWizard
Copy link

We'll tackle this issue when we get there, I think with a proper design of the base controllers it should be possible to create a base controller with logic for single action support, and one with multiple action support.

As for the routing, I am definitely against having to define or register hunderds of routes manually. If I want to do that, I'll use Laravel or something. Like in v1, registering should be an exception. And for me that isn't limited to routing, like I said, what I have against the Laravels and Symfony's of this world, is that you spend way to much time coding all sorts of housekeeping junk, time you should be spending on business logic. The fact that you can code up a controller that then "just works" is what people like about Fuel, and we should not do away with that, so we can design a Laravel clone...

@emlynwest
Copy link
Contributor Author

Fully agree with the routing issue there. On my TODO list as well to have "automagic" routing 😄

@sagikazarmark
Copy link

Automagic is nice...when it's a separate layer.

@WanWizard
Copy link

The first one is a functional requirement, the second a design requirement. Both are not mutually exclusive. 😄

@emlynwest
Copy link
Contributor Author

@sagikazarmark Don't worry, I'll set it up so it can be removed/replaced if wanted. I've been trying to avoid creating anything that forces a developer to do things in a certain way where possible.

@emlynwest emlynwest added this to the 2.0.0-beta1 milestone Apr 18, 2017
@bvnflatliner
Copy link

Don't kill multiple actions, please!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants