Fix review comments

Signed-off-by: Aleksandr Ivanov <[email protected]>
bloomberg · Jan 10, 2025 · 63f8d50 · 63f8d50
1 parent 2bc733c
commit 63f8d50
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 52 deletions.
diff --git a/src/applications/bmqstoragetool/m_bmqstoragetool_compositesequencenumber.cpp b/src/applications/bmqstoragetool/m_bmqstoragetool_compositesequencenumber.cpp
@@ -34,8 +34,8 @@ CompositeSequenceNumber::CompositeSequenceNumber()
 }
 
 CompositeSequenceNumber::CompositeSequenceNumber(
-    const unsigned int        leaseId,
-    const bsls::Types::Uint64 sequenceNumber)
+    unsigned int        leaseId,
+    bsls::Types::Uint64 sequenceNumber)
 : d_compositeSequenceNumber(leaseId, sequenceNumber)
 {
     // NOTHING
@@ -49,57 +49,63 @@ CompositeSequenceNumber::fromString(bool*              success,
     // PRECONDITION
     BSLS_ASSERT(success);
 
-    *success = false;
-
-    if (seqNumString.empty()) {
-        errorDescription << "Invalid input: empty string.";
-        return *this;  // RETURN
-    }
-
-    // Find the position of the separator
-    const size_t separatorPos = seqNumString.find('-');
-    if (separatorPos == bsl::string::npos) {
-        errorDescription << "Invalid format: no '-' separator found.";
-        return *this;  // RETURN
-    }
-
-    // Extract parts
-    const bsl::string firstPart  = seqNumString.substr(0, separatorPos);
-    const bsl::string secondPart = seqNumString.substr(separatorPos + 1);
-
-    // Convert parts to numbers
-    try {
-        size_t posFirst, posSecond;
-
-        const unsigned long       uLong     = bsl::stoul(firstPart, &posFirst);
-        const bsls::Types::Uint64 seqNumber = bsl::stoul(secondPart,
-                                                         &posSecond);
-
-        if (posFirst != firstPart.size() || posSecond != secondPart.size()) {
-            throw bsl::invalid_argument("");  // THROW
+    do {
+        if (seqNumString.empty()) {
+            errorDescription << "Invalid input: empty string.";
+            break;  // BREAK
         }
 
-        unsigned int leaseId = static_cast<unsigned int>(uLong);
-        if (uLong != leaseId) {
-            throw bsl::out_of_range("");  // THROW
+        // Find the position of the separator
+        const size_t separatorPos = seqNumString.find('-');
+        if (separatorPos == bsl::string::npos) {
+            errorDescription << "Invalid format: no '-' separator found.";
+            break;  // BREAK
         }
 
-        if (leaseId == 0 || seqNumber == 0) {
-            errorDescription << "Invalid input: zero values encountered.";
+        // Extract parts
+        const bsl::string firstPart  = seqNumString.substr(0, separatorPos);
+        const bsl::string secondPart = seqNumString.substr(separatorPos + 1);
+
+        // Convert parts to numbers
+        try {
+            size_t posFirst, posSecond;
+
+            const unsigned long       uLong = bsl::stoul(firstPart, &posFirst);
+            const bsls::Types::Uint64 seqNumber = bsl::stoul(secondPart,
+                                                             &posSecond);
+            if (posFirst != firstPart.size() ||
+                posSecond != secondPart.size()) {
+                errorDescription
+                    << "Invalid input: non-numeric values encountered.";
+                break;  // BREAK
+            }
+
+            unsigned int leaseId = static_cast<unsigned int>(uLong);
+            if (uLong != leaseId) {
+                errorDescription << "Invalid input: number out of range.";
+                break;  // BREAK
+            }
+
+            if (leaseId == 0 || seqNumber == 0) {
+                errorDescription << "Invalid input: zero values encountered.";
+                break;  // BREAK
+            }
+
+            d_compositeSequenceNumber = bsl::make_pair(leaseId, seqNumber);
+
+            *success = true;
             return *this;  // RETURN
         }
+        catch (const bsl::invalid_argument& e) {
+            errorDescription
+                << "Invalid input: non-numeric values encountered.";
+        }
+        catch (const bsl::out_of_range& e) {
+            errorDescription << "Invalid input: number out of range.";
+        }
+    } while (false);
 
-        d_compositeSequenceNumber = bsl::make_pair(leaseId, seqNumber);
-
-        *success = true;
-    }
-    catch (const bsl::invalid_argument& e) {
-        errorDescription << "Invalid input: non-numeric values encountered.";
-    }
-    catch (const bsl::out_of_range& e) {
-        errorDescription << "Invalid input: number out of range.";
-    }
-
+    *success = false;
     return *this;
 }
 

diff --git a/src/applications/bmqstoragetool/m_bmqstoragetool_compositesequencenumber.h b/src/applications/bmqstoragetool/m_bmqstoragetool_compositesequencenumber.h
@@ -57,8 +57,8 @@ class CompositeSequenceNumber {
 
     /// Create CompositeSequenceNumber from the specified `leaseId` and
     /// `sequenceNumber`
-    CompositeSequenceNumber(const unsigned int        leaseId,
-                            const bsls::Types::Uint64 sequenceNumber);
+    CompositeSequenceNumber(unsigned int        leaseId,
+                            bsls::Types::Uint64 sequenceNumber);
 
     // MANIPULATORS
 
@@ -80,9 +80,9 @@ class CompositeSequenceNumber {
     /// Return sequence number value.
     bsls::Types::Uint64 sequenceNumber() const;
 
-    /// Return the composite sequence number as a pair of primary lease Id and
-    /// sequence number.
-    bsl::pair<unsigned int, bsls::Types::Uint64>
+    /// Return the const reference to composite sequence number as a pair of
+    /// primary lease Id and sequence number.
+    const bsl::pair<unsigned int, bsls::Types::Uint64>&
     compositeSequenceNumber() const;
 
     /// Write the value of this object to the specified output `stream` in a
@@ -148,7 +148,7 @@ inline bsls::Types::Uint64 CompositeSequenceNumber::sequenceNumber() const
     return d_compositeSequenceNumber.second;
 }
 
-inline bsl::pair<unsigned int, bsls::Types::Uint64>
+inline const bsl::pair<unsigned int, bsls::Types::Uint64>&
 CompositeSequenceNumber::compositeSequenceNumber() const
 {
     return d_compositeSequenceNumber;

diff --git a/src/applications/bmqstoragetool/m_bmqstoragetool_parameters.cpp b/src/applications/bmqstoragetool/m_bmqstoragetool_parameters.cpp
@@ -422,12 +422,14 @@ Parameters::Parameters(const CommandLineArguments& arguments,
                 &success,
                 errorDescr,
                 arguments.d_seqNumLt);
+            BSLS_ASSERT(success);
         }
         if (!arguments.d_seqNumGt.empty()) {
             d_range.d_seqNumGt = CompositeSequenceNumber().fromString(
                 &success,
                 errorDescr,
                 arguments.d_seqNumGt);
+            BSLS_ASSERT(success);
         }
     }