Skip to content
This repository was archived by the owner on Jul 17, 2020. It is now read-only.

Feature: alias/inheritance types #25

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

Ocramius
Copy link
Owner

@Ocramius Ocramius commented Feb 2, 2015

This PR is about supporting inheritances/aliases:

class Foo { public $id; }
class Bar extends Foo {}

Since Bar is a subtype of Foo, requesting Foo to the IdentityMap should also look up in the Bar instances.

$bar = new Bar;

$bar->id = 123;

$identityMap->add($bar);

$this->assertSame($bar, $identityMap->getObject('Foo', 123));

The opposite is not true:

$foo = new Foo;

$foo->id = 123;

$identityMap->add($foo);

$this->assertNull($identityMap->getObject('Bar', 123));

I really could need some help on this, as I'm a bit confused about how to actually implement this logic in a performance-sensitive way.

I may also need to split the type resolver so that we actually loop through subclass/superclass types, or we duplicate the references in the identity map.

Thoughts anybody?

@zerkms
Copy link

zerkms commented Feb 2, 2015

(from a person who never used inheritance in DB/ORMs entities): it may be a problem when there is no constraint that guarantees that both Bar and Foo don't collide their id's.

@Ocramius
Copy link
Owner Author

Ocramius commented Feb 2, 2015

As an additional feedback, I'd add that this behavior is actually configurable case-by-case. Following cases are also possible:

  • aliasing (Foo\Bar === Baz\Tab, without any other logical connection between the two)
  • proxying (FooProxy === Foo)
  • inheritance mapping (Foo\Root ~= Foo\Child ~= Foo\Leaf, requires further inspection)

@mnapoli
Copy link

mnapoli commented Feb 2, 2015

This is probably very naive but indexing the object in $this->objectsByIdentityMap for each parent class?

i.e. $this->objectsByIdentityMap['Foo-123'] and $this->objectsByIdentityMap['Bar-123'] would point to the same object… Cheap lookup, but maybe not cheap to add()?

* @return string
*/
public function resolveType($type);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

EOL EOF

@Ocramius
Copy link
Owner Author

Ocramius commented Feb 2, 2015

@mnapoli yeah, that is one possible solution indeed, though it's not very efficient, isn't it?

@mnapoli
Copy link

mnapoli commented Feb 2, 2015

Not so efficient for memory usage but I have no idea if the difference is actually problematic.

But in any case you will have to get the parent classes. But is that kind of metadata cached? If it is, then I guess it's just a matter of get/set X times in the array instead of once (where X is the number of parent classes). Shouldn't be too bad?

@Ocramius
Copy link
Owner Author

Ocramius commented Feb 2, 2015

But is that kind of metadata cached?

Yep, I will cache everything possible

Shouldn't be too bad?

Probably better indeed (rather than slowing down the process for every type). I'll think about it, thanks :-)

@nikita2206
Copy link

If there's a constraint that they all share the same id space then I would always go to the root and use its name. So for Foo extends Bar, with Foo#123 and Bar#827 there would be an IdentityMap{ Bar{ 123: Foo#123, 827: Bar#827 }}

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.

5 participants