Skip to content

Comments

Implement IteratorAggregate in ObjectSet subclasses#7101

Merged
morozov merged 2 commits intodoctrine:4.4.xfrom
morozov:object-set-as-traversable
Aug 24, 2025
Merged

Implement IteratorAggregate in ObjectSet subclasses#7101
morozov merged 2 commits intodoctrine:4.4.xfrom
morozov:object-set-as-traversable

Conversation

@morozov
Copy link
Member

@morozov morozov commented Aug 23, 2025

Object sets may need to be iterated, but right now this is done by converting them to a list. This can be avoided if the set itself is made traversable.

This change is backward compatible, since since the ObjectSet interface being extended is @internal.

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Do we still need the toList() method then?

/** {@inheritDoc} */
public function getIterator(): Traversable
{
return new ArrayIterator($this->elements);
Copy link
Member

Choose a reason for hiding this comment

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

Using a generator might be a bit more lightweight.

Suggested change
return new ArrayIterator($this->elements);
yield from $this->elements;

/** {@inheritDoc} */
public function getIterator(): Traversable
{
return new ArrayIterator(array_values($this->elements));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return new ArrayIterator(array_values($this->elements));
foreach ($this->elements as $element) {
yield $element;
}

@morozov
Copy link
Member Author

morozov commented Aug 23, 2025

Do we still need the toList() method then?

Yes, we still use it in some places. E.g.

return new Table(
$this->name->toString(),
$this->columns->toList(),
$this->indexes->toList(),
$this->uniqueConstraints->toList(),
$this->foreignKeyConstraints->toList(),

@morozov morozov force-pushed the object-set-as-traversable branch from edcce80 to 3856e70 Compare August 23, 2025 20:46
@morozov morozov force-pushed the object-set-as-traversable branch from 3856e70 to d739bd8 Compare August 23, 2025 20:59
@derrabus
Copy link
Member

Makes sense.

@morozov morozov merged commit d6185ec into doctrine:4.4.x Aug 24, 2025
90 checks passed
@morozov morozov deleted the object-set-as-traversable branch August 24, 2025 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants