Patch second level cache association hydration

[holtkamp]

Fixed incorrect use of keys in DefaultRegion::getMultiple() and simplified overall mechanism

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3689

We use Jira to track the state of pull requests and the versions they got
included in.

[FabioBatSilva]

You can use a early return whenever you have a missing entry.

<?php
$result = array();

foreach ($collection->identifiers as $key) {
    $entryKey   = $this->getCacheEntryKey($key);
    $entryValue = $this->cache->fetch($entryKey);

    if ($entryValue === false) {
        return null;
    }

    $result[] = $entryValue;
}

return $result;

[holtkamp]

Yeah, I know and I know Doctrine uses a lot of early returns. Not my favorite though, I prefer the 'one return per function' approach.

Not sure whether it is a formal Doctrine code convention?

[Ocramius]

@holtkamp please use an early return: it's less confusing to follow

[guilhermeblanco]

@holtkamp it's not a convention from Doctrine, but we should make it.
There is clear advantage of applying early returns; you increase code readability, reduce the cyclomatic complexity of your algorithm and also have fewer bytecode instructions to follow.

[holtkamp]

Ok, clear. A senior colleague (Java guy) once taught me to use only one
return per function to ease up debugging (only one breakpoint per function
required), since then I adopted it as a daily habit. But it is fine by me,
I regard more as a 'style' thingy ;).

Just pushed and update of the PR.

On 14 April 2015 at 16:15, Guilherme Blanco [email protected]
wrote:

In lib/Doctrine/ORM/Cache/Region/DefaultRegion.php
#1382 (comment):

@@ -100,30 +102,28 @@ public function get(CacheKey $key)
*/
public function getMultiple(CollectionCacheEntry $collection)
{

- $keysToRetrieve = array();

•    foreach ($collection->identifiers as $index => $key) {
  

•        $keysToRetrieve[$index] = $this->name . '_' . $key->hash;
  

- }

•    $items = array_filter(
  

•        array_map([$this->cache, 'fetch'], $keysToRetrieve),
  

•        function ($retrieved) {
  

•            return false !== $retrieved;
  

•    $result = array();
  

@holtkamp https://github.com/holtkamp it's not a convention from
Doctrine, but we should make it.
There is clear advantage of applying early returns; you increase code
readability, reduce the cyclomatic complexity of your algorithm and also
have fewer bytecode instructions to follow.

—
Reply to this email directly or view it on GitHub
https://github.com/doctrine/doctrine2/pull/1382/files#r28330003.

[guilhermeblanco]

@holtkamp read and give your friend to read this: http://www.xpteam.com/jeff/writings/objectcalisthenics.rtf

Another place you could read is this: https://github.com/object-calisthenics/phpcs-calisthenics-rules

[guilhermeblanco]

👍

[holtkamp]

Would this bugfix justify a new release (for example 2.5.1)? Looking forward to using the SLC...

[Ocramius]

It landed in the 2.5 branch at 3e6c6af

We have 2 or 3 bad bugs that require attention, then I'm tagging 2.5.1

[holtkamp]

@Ocramius, it has been an month now, isn't a 2.5.1 patch version justified to at least fix 1 serious bug? For the other mentioned bugs new patch versions can be used according to www.semver.org, right?

[stof]

@Ocramius it would be great to fix the tests in the 2.5 branch btw

[holtkamp]

@stof what (broken) tests are you referring to? Is a 2.5.1 version still being aimed for or... @Ocramius what 2 to 3 bad bugs were you referring to that impede a 2.5.1 patch version? Maybe I can help.