BewareMyPower commented on code in PR #21:
URL: https://github.com/apache/pulsar-client-cpp/pull/21#discussion_r996857527
##########
lib/ConsumerImplBase.h:
##########
@@ -20,23 +20,38 @@
#define PULSAR_CONSUMER_IMPL_BASE_HEADER
#include <pulsar/Message.h>
#include <pulsar/Consumer.h>
-
+#include "HandlerBase.h"
+#include <queue>
#include <set>
namespace pulsar {
class ConsumerImplBase;
+class HandlerBase;
typedef std::weak_ptr<ConsumerImplBase> ConsumerImplBaseWeakPtr;
-class ConsumerImplBase {
+class OpBatchReceive {
public:
- virtual ~ConsumerImplBase() {}
+ OpBatchReceive();
+ explicit OpBatchReceive(const BatchReceiveCallback& batchReceiveCallback);
+ const BatchReceiveCallback batchReceiveCallback_;
+ const int64_t createAt_;
+};
+
+class ConsumerImplBase : public HandlerBase, public
std::enable_shared_from_this<ConsumerImplBase> {
Review Comment:
Just now I found another problem, which is caused by the multiple
inheritance. Though it's not caused by your PR, it's a coding problem from
early times.
IMO, the `HandlerBase` should inherit `enable_shared_from_this`, see
https://github.com/apache/pulsar-client-cpp/blob/c1a98084ba6c41c8b8e4f822b0224a25d173f258/lib/HandlerBase.cc#L86
and
https://github.com/apache/pulsar-client-cpp/blob/c1a98084ba6c41c8b8e4f822b0224a25d173f258/lib/HandlerBase.cc#L141
Obviously, it's against the OOP rule. However, since `HandlerBase` doesn't
inherit `enable_shared_from_this`, there is no way to determine if the
`HandlerBase` object is valid in the callback of its timer. That's because the
derived classes (`ProducerImpl`, `ConsumerImpl`, etc.) have already inherited
the `enabled_shared_from_this`.
If you inherited `enable_shared_from_this` for `ConsumerImplBase`, there
would be impossible to do that for `HandlerBase`. I am going to fix this
problem in my incoming PR.
--
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]
