BewareMyPower commented on a change in pull request #14604:
URL: https://github.com/apache/pulsar/pull/14604#discussion_r821734237
##########
File path: pulsar-client-cpp/include/pulsar/MessageId.h
##########
@@ -28,6 +28,7 @@
namespace pulsar {
class MessageIdImpl;
+typedef std::shared_ptr<MessageIdImpl> MessageIdImplPtr;
Review comment:
Is this `typedef` used anywhere?
##########
File path: pulsar-client-cpp/include/pulsar/MessageId.h
##########
@@ -44,6 +45,12 @@ class PULSAR_PUBLIC MessageId {
*/
explicit MessageId(int32_t partition, int64_t ledgerId, int64_t entryId,
int32_t batchIndex);
+ /**
+ * Construct the ChunkMessageId with first and last message id's param
+ */
+ explicit MessageId(int32_t firstPartition, int64_t firstLedgerId, int64_t
firstEntryId, int32_t firstBatchIndex,
+ int32_t lastPartition, int64_t lastLedgerId, int64_t
lastEntryId, int32_t lastBatchIndex);
Review comment:
The code readability of constructor with 8 parameters is bad. (Though I
think the current constructor with 4 parameters is also not good).
My suggestion is like
```c++
/**
* Construct a chunked message ID.
*/
MessageId(const MessageId& firstMessageId, const MessageId& lastMessageId);
```
--
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]
