Skip to content

Add caching to toArray method #2

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

Merged
merged 3 commits into from
Nov 11, 2013
Merged

Add caching to toArray method #2

merged 3 commits into from
Nov 11, 2013

Conversation

garoevans
Copy link
Contributor

Reflection can be slow on occasions and this optimization should be beneficial if toArray is being called more than once per request.

Reflection can be slow on occasions and this optimization should be beneficial if `toArray` is being called more than once per request.
@mnapoli
Copy link
Member

mnapoli commented Nov 11, 2013

Nice!

But might as well cache the constants no?

Something like:

$classname = get_called_class();

if (! array_key_exists($calledClass, self::$constantsCache)) {
    $reflectionClass = new \ReflectionClass($calledClass);
    self::$constantsCache[$classname] = $reflection->getConstants();
}

return self::$constantsCache[$classname];

* Store instantiated reflection objects in a static cache.
* @var array
*/
protected static $reflectionCache = array();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be private, I see no use to making it accessible to subclasses

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely agree. Let me get this sorted.

- Cache constants rather than reflection objects
- Make static cache private
@garoevans
Copy link
Contributor Author

Editing in github is no fun, but typo has gone and travis looks OK

mnapoli added a commit that referenced this pull request Nov 11, 2013
Add caching to `toArray` method
@mnapoli mnapoli merged commit b52c2f2 into myclabs:master Nov 11, 2013
@mnapoli
Copy link
Member

mnapoli commented Nov 11, 2013

Awesome, thanks for contributing.

This has been released as 1.2.1.

mnapoli pushed a commit that referenced this pull request Feb 4, 2019
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.

2 participants