ChangeSet icon indicating copy to clipboard operation
ChangeSet copied to clipboard

Feature: alias/inheritance types

Open Ocramius opened this issue 10 years ago • 7 comments

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?

Ocramius avatar Feb 02 '15 01:02 Ocramius

(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.

zerkms avatar Feb 02 '15 02:02 zerkms

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)

Ocramius avatar Feb 02 '15 02:02 Ocramius

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()?

mnapoli avatar Feb 02 '15 02:02 mnapoli

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

Ocramius avatar Feb 02 '15 02:02 Ocramius

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?

mnapoli avatar Feb 02 '15 02:02 mnapoli

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 :-)

Ocramius avatar Feb 02 '15 02:02 Ocramius

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 }}

nikita2206 avatar Feb 02 '15 13:02 nikita2206