Update AmqpMember.php #43
Conversation
typos
| * @param string $consumerTag | ||
| */ | ||
| public function __construct(Connection $conn, $consumerTag = null) | ||
| public function __construct(IConnection $conn, $consumerTag = null) |
There was a problem hiding this comment.
That would break the contact of the interface. Why is this needed?
There was a problem hiding this comment.
I need use SSLConnection (https://github.com/php-amqplib/php-amqplib/blob/master/PhpAmqpLib/Connection/AMQPSSLConnection.php) instead of Connection.
There was a problem hiding this comment.
Well, I can understand that, but this is not a good way to solve it.
There was a problem hiding this comment.
Why? What is the correct way? Please.
There was a problem hiding this comment.
AmqpMember and subclasses of it are using some methods of the given $conn instance. With this change, you create dependency on interface, that doesn't require those methods -> it's breaking the contract of the interface. When in fact, you depend on broader interface.
There was a problem hiding this comment.
I understand. But then I can not think of a way to use class SSLConnection? Instead interface use abstract class AbstractConnection?
PS: sorry za angličtinu... můj skill není moc cool.
There was a problem hiding this comment.
Je potřeba se podívat, jaky metody to používá :) Pokud AmqpMember ani zadni jeho potomci nepotřebují nic z IConnection, může se tam v pohodě hodit AbstractConnection.
Na druhou stranu, bude ale potřeba podědit SSLConnection a přidat tam věci z Kdyby\RabbitMq\Connection, jinak přestane fungovat zbytek knihovny. Možná bych z toho dokonce udělal traitu.
There was a problem hiding this comment.
Momentálně to co využívám mi spolkne i typehint s tím interface. Ale chápu problém.
Teď přemýšlím jak jsi to přesně myslel.
Ideálně, aby v neonu pokud použiju třeba ssl:true se použilo Connection s funkčností z SSLConnection. Jestli si pamatuji, tak tvůj kód očekává v typehintech instance tvé Connection.
Knihovně by tak mělo být jedno jakou používá instanci. Jestli AMQPLazyConnection nebo AMQPSSLConnection. Nejbližší rodič obou dvou je AMQPStreamConnection a pak AbstractConnection.
Musel bych to asi promyslet, která cesta bude lepší... pokud to nepředěláš samozřejmě ty.
Traitu jak jsem psal úplně nevím jak do toho zakomponovat.
Jestli ji vložit do Connection a na základě parametrů, které dostane od DI volat vytváření contextu. Protože jediné co je tam navíc koukám je metoda create_ssl_context.
typos