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

Added syntactic sugar for route collector #109

Closed
wants to merge 1 commit into from

Conversation

ragboyjr
Copy link
Contributor

Added simpler methods for syntactic sugar for defining routes.

When defining a lot of routes, this saves a lot of typing overhead.

Added simpler methods for syntactic sugar for defining routes.
@nikic
Copy link
Owner

nikic commented Jul 30, 2016

Any opinions on this? /cc @bwoebi

Seems like a reasonable addition to simplify standalone use.

@bwoebi
Copy link
Contributor

bwoebi commented Jul 30, 2016

In this case, I'd honestly just use __call() magic. Otherwise the next one comes and wants this method supported or that one and next year a RFC will introduce even another method...

@nikic
Copy link
Owner

nikic commented Jul 30, 2016

@bwoebi Not a big fan of call magic -- it is prone to hide mistakes, e.g. $r->addRuote('GET', '/foo', 'h') would actually add the route ('ADDRUOTE', 'GET', '/foo'). The methods proposed here (maybe with addition of HEAD) seem to be the only request methods in broad use, at least as far as HTTP is concerned.

@bwoebi
Copy link
Contributor

bwoebi commented Jul 30, 2016

@nikic doing $r->addRoute("GETT", "/foo", "h") is just as prone to errors. I do not see a significant disadvantage here.

@assertchris
Copy link

Could check for method spelling (inside __call), using in_array. Of course all this adds overhead...

@ragboyjr
Copy link
Contributor Author

Personally, I prefer to not use the __call magic method in order because it's less obvious and a bit messier to type hint/document IMO.

@ragboyjr
Copy link
Contributor Author

ragboyjr commented Sep 3, 2016

@nikic any chance of getting this merged? Should I add any more methods?

@nikic
Copy link
Owner

nikic commented Sep 3, 2016

Sorry for the delay. Merged with a head() method and a basic test: 7b15535

@nikic nikic closed this Sep 3, 2016
@ragboyjr
Copy link
Contributor Author

ragboyjr commented Sep 4, 2016

@nikic thanks!

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.

None yet

4 participants