cbornet commented on PR #11:
URL:
https://github.com/apache/pulsar-client-reactive/pull/11#issuecomment-1291725975
> I disagree with the current approach of adding RxJava support. RxJava
shouldn't be any exception. The pulsar-client-reactive-api module shouldn't
know anything about RxJava at compile time. As I mentioned in my previous
comments, adding support for a single RS implementation as an exceptional case
is not a scalable solution. It is necessary to make it pluggable so that
support for any RS implementation library can be added even without
contributing the changes to pulsar-client-reactive.
The intent is not to have RxJava as an exception. The intent is to have a
pluggable implementation with RxJava and Reactor already plugged-in. But
nothing prevents from adding your own impl.
There are several problems with the adapt approach:
* The signature is `<R extends ReactiveMessageReader> R adapt(Class<R>
adaptedReaderType)` but what we want to return is `ReactiveMessageReader<T>`.
It works with type erasure but we get an unchecked assignment warning (see
ReactiveMessageSenderE2ETest::shouldSendMessageToTopic that uses it in the PR.
* We don't know how to build the adaptedReaderType instance. There's no
contract for it. Maybe could have an implicit contract where we call the
constructor with the `this` as parameter. Something like
```java
default <R extends ReactiveMessageSender> R adapt(Class<R>
adaptedSenderType) throws Exception {
return
adaptedSenderType.getDeclaredConstructor(ReactiveMessageSender.class).newInstance(this);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
