Added support for route filters to the application and dispatcher classes #12

Merged
merged 2 commits into from Jun 29, 2011

Conversation

Projects
None yet
2 participants
Contributor

elazar commented Jun 8, 2011

Added support for route filters to the application and dispatcher classes, updated the README and unit tests accordingly

Added support for route filters to the application and dispatcher cla…
…sses, updated the README and unit tests accordingly
Owner

whatthejeff commented Jun 20, 2011

What do you think about using the return value from the callback instead of passing by reference?

Contributor

elazar commented Jun 20, 2011

I considered that, but all the code invoking the callbacks would be doing is reassigning the return value back to the same variable. I don't know if the Zend Engine is smart enough to realize this and reuse memory already allocated for that variable value rather than allocating additional memory.

Owner

whatthejeff commented Jun 20, 2011

I don't believe any additional memory is allocated in these types of situations. I will run some tests against your code with xdebug tonight and post my findings.

Owner

whatthejeff commented Jun 21, 2011

Real memory usage seems to be the same using both methods. Xdebug actually reports slightly higher memory usage for the pass-by-reference solution, though I'm not sure why.

Contributor

elazar commented Jun 21, 2011

In that case, I don't have a problem moving the code to return its result rather than modify a passed-by-reference variable. Suppose I should have done that Xdebug analysis myself. Thanks for being thorough. :) Would you like me to commit that change, or do you want to handle it? I'm fine with either.

Owner

whatthejeff commented Jun 21, 2011

You can do it. I'll have some time tonight to pull in the changes. Thanks again!

Owner

whatthejeff commented Jun 28, 2011

I'll merge this tonight. Thanks again @elazar :)

whatthejeff added a commit that referenced this pull request Jun 29, 2011

Merge pull request #12 from elazar/route-filters
Added support for route filters to the application and dispatcher classes

@whatthejeff whatthejeff merged commit 4caae2b into whatthejeff:master Jun 29, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment