Automatically set lookupFields based on the entities identifier fields

[lsmith77]

This is an untested change and the PR is more meant to start a discussion on this since this is kind of a BC break.

Inside the class itself the following code uses the lookupFields:

            if (!empty($this->lookupFields)) {
                $lookupConditions = array();
                foreach ($this->lookupFields as $fieldName) {
                    $lookupConditions[$fieldName] = $item[$fieldName];
                }

                $object = call_user_func($this->lookupMethod, $lookupConditions);
            } else {
                $object = $this->objectRepository->find(current($item));
            }

The else case would now of course never be reached. Now $index defaults to null in the constructor. We could of course make it even more flexible by optionally also supporting a boolean to determine if to use the old approach or if to use the new approach added in this PR.

wdyt?

[lsmith77]

@ddeboer ping :)

[Baachi]

Hey @lsmith77,

nice addition! But i'm not really sure if a 5th parameter is such a good design.
WDYT about to extract the lookup logic to a seperate class, which is than passed to the constructor.

[lsmith77]

@Baachi how to handle BC in such a case?

Maybe adding a factory method for the different use cases?
DoctrineWriter::createX() ?

[ddeboer]

WDYT about to extract the lookup logic to a seperate class, which is than passed to the constructor.

Good idea: see #11.

While it is a slight BC break, I’m okay with removing the ->find(current($item)) and changing the default behaviour to using the identifier field names, because it’s more explicit than relying on the first value in $item (whatever that may be) to match the primary key.

So @lsmith77, could you please add a test case?

[ddeboer]

Possibly fixes portphp/portphp#50.