jai1 commented on a change in pull request #1322: Use private impl for 
MessageId in c++ client
URL: https://github.com/apache/incubator-pulsar/pull/1322#discussion_r172706345
 
 

 ##########
 File path: pulsar-client-cpp/lib/MessageId.cc
 ##########
 @@ -75,38 +82,81 @@ void MessageId::serialize(std::string& result) const {
 /**
  * Deserialize a message id from a binary string
  */
-boost::shared_ptr<MessageId> MessageId::deserialize(const std::string& 
serializedMessageId) {
+MessageId MessageId::deserialize(const std::string& serializedMessageId) {
     proto::MessageIdData idData;
     if (!idData.ParseFromString(serializedMessageId)) {
         throw "Failed to parse serialized message id";
     }
 
-    return boost::make_shared<BatchMessageId>(idData.ledgerid(), 
idData.entryid(), idData.batch_index());
+    return MessageId(idData.partition(), idData.ledgerid(), idData.entryid(), 
idData.batch_index());
 }
 
+int64_t MessageId::ledgerId() const { return impl_->ledgerId_; }
+
+int64_t MessageId::entryId() const { return impl_->entryId_; }
+
+int32_t MessageId::batchIndex() const { return impl_->batchIndex_; }
+
+int32_t MessageId::partition() const { return impl_->partition_; }
+
 #pragma GCC visibility push(default)
+
 std::ostream& operator<<(std::ostream& s, const pulsar::MessageId& messageId) {
-    s << '(' << messageId.ledgerId_ << ',' << messageId.entryId_ << ',' << 
messageId.partition_ << ')';
+    s << '(' << messageId.impl_->ledgerId_ << ',' << messageId.impl_->entryId_ 
<< ','
+      << messageId.impl_->batchIndex_ << ',' << messageId.impl_->partition_ << 
')';
     return s;
 }
 
 bool MessageId::operator<(const MessageId& other) const {
-    if (ledgerId_ < other.ledgerId_) {
+    if (impl_->ledgerId_ < other.impl_->ledgerId_) {
+        return true;
+    } else if (impl_->ledgerId_ > other.impl_->ledgerId_) {
+        return false;
+    }
+
+    if (impl_->entryId_ < other.impl_->entryId_) {
+        return true;
+    } else if (impl_->entryId_ > other.impl_->entryId_) {
+        return false;
+    }
+
+    if (impl_->batchIndex_ < other.impl_->batchIndex_) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+bool MessageId::operator<=(const MessageId& other) const {
+    if (impl_->ledgerId_ < other.impl_->ledgerId_) {
 
 Review comment:
   Logic is correct but it's easier to have 
   return (*this < other) || (*this == other)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to