-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Add the Countable interface to RouteCollection. #4642
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
Conversation
*/ | ||
public function count() | ||
{ | ||
return count($this->routes->all()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indentation here. It should use 4 spaces
You Symfony people and your gratuitous whitespace... :-P Both fixed and rebased. |
@Crell running php-cs-fixer on your changes will prevent you from being stoffed :P about CS |
@mvrhov depends. The CS fixer will replace tabs with 4 spaces, but it will not fix it if using 2 spaces instead of 4. it cannot fix everything |
@Crell what is your use case ? |
I'm not opposed to the change but what's the use case? |
This is semi related to ideas we have for the dynamic router in the CMF. The 2 main reasons for the existence of the dynamic router are:
Now in some cases despite 1) the users might still want to dump key routes or even all routes as part of some deployment process to mod_rewrite or PHP when moving content from a staging database to a production database for better performance and reduced load on the database server. Also even with the dynamic router it would be good to have tools available like But for both the use case of dumping some/all routes or when listing routes it might be necessary to implement some stop gap solutions to first check if this wouldn't lead to a too big collection. So it could be useful to be able to use whatever efficient solution the dynamic data store has for determining the count before starting to actually read the data from the data source. |
Commits ------- c350944 Add the Countable interface to RouteCollection. Discussion ---------- Add the Countable interface to RouteCollection. Since RouteCollection is a fancy-pants array of Routes, and it already is iterable, it would be nice if it were also countable. There may be optimizations to be had here, but I figure this is a decent start. There should be no BC breaks here, just some DX convenience. --------------------------------------------------------------------------- by Crell at 2012-06-23T17:01:43Z You Symfony people and your gratuitous whitespace... :-P Both fixed and rebased. --------------------------------------------------------------------------- by travisbot at 2012-06-23T17:25:31Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1689898) (merged 6b588eb3 into 0d4b02e). --------------------------------------------------------------------------- by travisbot at 2012-06-23T17:26:06Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1689902) (merged abd74cf5 into 0d4b02e). --------------------------------------------------------------------------- by travisbot at 2012-06-23T18:10:57Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1689975) (merged c350944 into 0d4b02e). --------------------------------------------------------------------------- by mvrhov at 2012-06-24T07:11:12Z @Crell running [php-cs-fixer](http://cs.sensiolabs.org/) on your changes will prevent you from being stoffed :P about CS --------------------------------------------------------------------------- by stof at 2012-06-24T09:34:24Z @mvrhov depends. The CS fixer will replace tabs with 4 spaces, but it will not fix it if using 2 spaces instead of 4. it cannot fix everything --------------------------------------------------------------------------- by vicb at 2012-06-24T17:14:14Z @Crell what is your use case ? --------------------------------------------------------------------------- by fabpot at 2012-06-25T06:59:00Z I'm not opposed to the change but what's the use case? --------------------------------------------------------------------------- by lsmith77 at 2012-06-25T11:22:01Z This is semi related to ideas we have for the dynamic router in the CMF. The 2 main reasons for the existence of the dynamic router are: 1) the fact that end users should be able to define new routes 2) the fact that there might just be too many routes to dump those efficiently to PHP Now in some cases despite 1) the users might still want to dump key routes or even all routes as part of some deployment process to mod_rewrite or PHP when moving content from a staging database to a production database for better performance and reduced load on the database server. Also even with the dynamic router it would be good to have tools available like ``app/console router:debug``. But for both the use case of dumping some/all routes or when listing routes it might be necessary to implement some stop gap solutions to first check if this wouldn't lead to a too big collection. So it could be useful to be able to use whatever efficient solution the dynamic data store has for determining the count before starting to actually read the data from the data source.
Not the use case I'd been looking at, but still a valid one. Although, process question. If Travis just said above that this PR failed tests (although I'm unclear why), why was it merged anyway? (I'm used to Drupal where if the testbot doesn't like it, it doesn't happen. Period.) |
Since RouteCollection is a fancy-pants array of Routes, and it already is iterable, it would be nice if it were also countable.
There may be optimizations to be had here, but I figure this is a decent start. There should be no BC breaks here, just some DX convenience.