Skip to content
This repository was archived by the owner on Sep 16, 2021. It is now read-only.

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Jul 2, 2014

@dantleech
Copy link
Member

For me this is more of a desirable feature, not a "blocker" - it needs a rebase ..

@wouterj
Copy link
Member Author

wouterj commented Jul 21, 2014

It's less than a blocker, but more than a "nice to have" imo :)

Copy link
Member

Choose a reason for hiding this comment

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

I find this line extremely difficuly to understand, I would prefer something more structured like the following:

(infact I only understood what it did after writing the following code)

  $resolvedMetadata = null;

  // if there is cache
  if (null !== $this->cache) {
      $metadata = $this-loadClassMetadataFromCache($reflection);

      if ($metadata->isFresh) {
          $resolvedMetadata = $this->getCachedMetadata($reflection);
      } else {
          $resolvedMetadata = $this->loadMetadataForClass($class, $metadata, $addedClasses);

          if (null === $resolvedMetadata) {
              $resolvedMetadata = $this->resolveMetadata($class, null, $addedClasses);
          }

          if (null === $resolvedMetadata) {
              return null; // throw exception??
          }

          $this->putClassMetadataInCache($resolvedMetadata);
      }

      $this->resolvedMetadata[$class] = $resolvedMetadata;
  }

  return $this->resolvedMetadata[$class];

@wouterj wouterj closed this Mar 24, 2015
@wouterj wouterj deleted the caching branch March 24, 2015 10:51
@dantleech
Copy link
Member

Why close this? Would we do this in a different way now?

@wouterj
Copy link
Member Author

wouterj commented Mar 24, 2015

This PR has been open for very long and I can no longer follow the logic in here. Maybe I'll write caching again in some other PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants