refactor(http): remove usage of MatchedRoute

[brendt]

No description provided.

[brendt]

This PR currently fails, but I pushed it because I wanted to gather some opinions. @blackshadev you've been working a lot in the router, so I figured I'd ask you.

My goal is to see whether it's possible to get rid of MatchedRoute, and instead use a tagged singleton for it. MatchRoute was kind of a hack I did a long time ago, and I'd like it to be cleaned up.

Where I'm struggling atm is that $params on Route mean something different than $params on MatchedRoute. For MatchedRoute they hold the actual values, while for Route they are a list of param names.

Would it make sense in continuing this refactor and try to merge the two together? Maybe have Route::paramNames and Route::paramValues to make a clearer distinction? What do you think about that?

[blackshadev]

This PR currently fails, but I pushed it because I wanted to gather some opinions. @blackshadev you've been working a lot in the router, so I figured I'd ask you.

My goal is to see whether it's possible to get rid of MatchedRoute, and instead use a tagged singleton for it. MatchRoute was kind of a hack I did a long time ago, and I'd like it to be cleaned up.

Where I'm struggling atm is that $params on Route mean something different than $params on MatchedRoute. For MatchedRoute they hold the actual values, while for Route they are a list of param names.

Would it make sense in continuing this refactor and try to merge the two together? Maybe have Route::paramNames and Route::paramValues to make a clearer distinction? What do you think about that?

I am a bit in doubt on this actually, I will state my positive and negative opinion and draw a conclusion from that, of course, it is up to you what you do with it, it is still solely my opinion.

Generally I like for my classes to have a singular meaning and I like them even more to not change over the duration of my program, or at least as less as possible. My reasoning for this is makes it easier to talk about these objects by there name. For example, If we talk about "the route" does that mean the a route defined by an attribute, so more of a definition? Does that mean the current route with your parameters resolved on it? Your idea, goes fully against that, and actually makes it harder to talk about "the route" as a concept.
The positive thing is: I think, from an user perspective (the user of the tempest framework) this makes more sense in terms of terminology, If you Inject a route you generally mean the current route. And why would you care about the "MatchedRoute" , the whole concept of matching should be transparent for the user.

So to conclude: I think you shouldn't modify state on the Route and use it for 2 different concept: the definition and the "current route" . What I do think is we should focus on the framework user's view of the world instead of the maintainers view.

If I were to implement my thoughts into code: I would introduce a RouteDefinition class which holds the definition (previously known as Route), and introduce a Route class which is instantiated after matching the route using the matched route definition and the parameters. The new Route class is comparable to the previous MatchedRoute with one important distinction (besides naming), I would copy the properties over from the RouteDefinition which are relevant to the user and make them public readonly properties. Namely, the controller/action, a route name (if we have it), uri, method, all the things. But not the previous Route::$params and all the regex consts. These are more of an internal thing for the matcher . This way we have a clear separation between internal RouteDefinition which include regex things , extra bookkeeping, splitting, and the framework user view "Route.

[brendt]

You make a very good point! Thanks for pitching in, I appreciate that. Based on your reply, I don't think I should go ahead with this PR — and I'm totally ok with that.

I would introduce a RouteDefinition class which holds the definition (previously known as Route)

I don't mind the idea, although I also wanted #[Route] to be available as an attribute. OTOH, I reckon no one will use #[Route] directly; they would always opt for #[Get], #[Post], etc. So it might be a matter of me just letting go of the idea that #[Route] can be used directly. Need to think about that.

Anyway, gonna close this PR, thanks for the input!