Interesting bug

[ghost]

I just came across an interesting bug I thought you might like.

I have a Set that is being created using an array like this pseudo code

foreach ($users as &$userId) {
    $userId = UserId::fromString($userId->user_uuid);
}

$result = new Set($users);

UserId implements Hashable.

When I come to add another UserId to the Set, even if the hash functions return the same value, and the equals functions return true, the new UserId is added to the Set.

Set after loop and before add:

class Ds\Set#41 (3) {
  public ${0} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#40 (1) {
    private $identity =>
    string(36) "4f220a85-1b2f-40f6-91c8-5879a8331c9f"
  }
  public ${1} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#42 (1) {
    private $identity =>
    string(36) "5e006610-5ed4-4393-a6e7-57857ac5deec"
  }
  public ${2} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#43 (1) {
    private $identity =>
    string(36) "b6f23619-dfca-423f-b598-3878a30abbc1"
  }
}

Set after loop and after add

class Ds\Set#41 (4) {
  public ${0} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#40 (1) {
    private $identity =>
    string(36) "4f220a85-1b2f-40f6-91c8-5879a8331c9f"
  }
  public ${1} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#42 (1) {
    private $identity =>
    string(36) "5e006610-5ed4-4393-a6e7-57857ac5deec"
  }
  public ${2} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#43 (1) {
    private $identity =>
    string(36) "b6f23619-dfca-423f-b598-3878a30abbc1"
  }
  public ${3} =>
  class ThoughtRiver\Review\Domain\Model\User\UserId#32 (1) {
    private $identity =>
    string(36) "5e006610-5ed4-4393-a6e7-57857ac5deec"
  }
}

Notice indexes 1 and 3

If I change the loop to the following:

$result = new Set;

foreach ($users as $userId) {
    $result->add(UserId::fromString($userId->user_uuid));
}

Then I cannot add a new UserId to the Set when the Set already contains an object that equals considers to be true.

How weird?

[rtheunissen]

So strange. :thinking:

I'll take a look asap. Thanks for the report. :)

[rtheunissen]

I have a feeling it's because we're not dereferencing consistently. I don't think it's add vs constructor, but the fact that you're using references in the first case.

What's your code for hash and equals?

[ghost]

It will definitely be because of the references.

    public function equals($obj): bool
    {
        return get_class($this) === get_class($obj)
            && $this->hash() === $obj->hash();
    }

    public function hash()
    {
        return (string) $this;
    }

    public function __toString(): string
    {
        return (string) $this->identity;
    }

Just a simple set of functions as it's a uuid.

[rtheunissen]

I think this has now been fixed in the extension?

[ghost]

Great if it has! I will try to reproduce this (the code has moved on since I logged it).

[designermonkey]

I know it's been quite a while, sorry I didn't get to this earlier.

I've just done an overhaul of our tests:

• Run on a system using the polyfill: Everything is ok.
• Run on a system using the extension: Same result as above.

To save on work, I've changed the code to not use references as described above. I thought you would want to know that there is still a references issue lurking in there somewhere.

[rtheunissen]

Thanks @designermonkey, I'll try to add a failing test for this.