imap icon indicating copy to clipboard operation
imap copied to clipboard

MailboxIterface: Typehint to interface instead of implementation

Open homersimpsons opened this issue 4 years ago • 5 comments

Prefer MessageIteratorInterface instead of MessageIterator.

This should not be a breaking change as MessageIterator implements MessageIteratorInterface

homersimpsons avatar Jul 28 '20 13:07 homersimpsons

Nice, could you also update Mailbox as well please?

Slamdunk avatar Jul 28 '20 13:07 Slamdunk

Thank you for this blazing fast response.

I did update it.

The problem is that Mailbox::prepareMessageIds needs \ArrayIterator::getArrayCopy which is not in MailboxInterface. What is the expected fix ?

homersimpsons avatar Jul 28 '20 14:07 homersimpsons

The issue is that ArrayIterator::getArrayCopy is not part of any interface.

Now I remember this is the reason why I type-hinted against MessageIterator and not MessageIteratorInteface.

I see no solution to implement any part of this PR.

Slamdunk avatar Jul 29 '20 13:07 Slamdunk

Actually we could have something along

if ($messageIds instanceof MessageIterator) {
    $messageIds = $messageIds->getArrayCopy();
} elseif ($messageIds instanceof MessageIteratorInterface) {
    $messageIds = iterator_to_array($messageIds, false);
}

homersimpsons avatar Jul 29 '20 20:07 homersimpsons

That wouldn't respect proper type covariance in type hints.

We should set MessageIteratorInterface to extend Traversable interface, to we can easily use iterator_to_array as you suggest

Slamdunk avatar Jul 30 '20 07:07 Slamdunk