[FOLD] Address feedback from Scott Schurr

XRPLF · Feb 23, 2021 · 6c6f7a9 · 6c6f7a9
1 parent 8167c59
commit 6c6f7a9
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 13 deletions.
diff --git a/src/ripple/basics/RangeSet.h b/src/ripple/basics/RangeSet.h
@@ -119,7 +119,7 @@ to_string(RangeSet<T> const& rs)
     @return True on successfully converting styled string
 */
 template <class T>
-bool
+[[nodiscard]] bool
 from_string(RangeSet<T>& rs, std::string const& s)
 {
     std::vector<std::string> intervals;

diff --git a/src/ripple/nodestore/ShardInfo.h b/src/ripple/nodestore/ShardInfo.h
@@ -37,13 +37,26 @@ class ShardInfo
     {
     public:
         Incomplete() = delete;
-        Incomplete(ShardState state_, std::uint32_t percentProgress_)
-            : state(state_), percentProgress(percentProgress_)
+        Incomplete(ShardState state, std::uint32_t percentProgress)
+            : state_(state), percentProgress_(percentProgress)
         {
         }
 
-        ShardState const state;
-        std::uint32_t const percentProgress;
+        [[nodiscard]] ShardState
+        state() const noexcept
+        {
+            return state_;
+        }
+
+        [[nodiscard]] std::uint32_t
+        percentProgress() const noexcept
+        {
+            return percentProgress_;
+        }
+
+    private:
+        ShardState state_;
+        std::uint32_t percentProgress_;
     };
 
 public:

diff --git a/src/ripple/nodestore/Types.h b/src/ripple/nodestore/Types.h
@@ -57,7 +57,7 @@ using Batch = std::vector<std::shared_ptr<NodeObject>>;
 }  // namespace NodeStore
 
 /** Shard states. */
-enum class ShardState {
+enum class ShardState : std::uint32_t {
     acquire,     // Acquiring ledgers
     complete,    // Backend is ledger complete, database is unverified
     finalizing,  // Verifying database

diff --git a/src/ripple/nodestore/impl/ShardInfo.cpp b/src/ripple/nodestore/impl/ShardInfo.cpp
@@ -43,7 +43,7 @@ ShardInfo::incompleteToString() const
         for (auto const& [shardIndex, incomplete] : incomplete_)
         {
             result += std::to_string(shardIndex) + ":" +
-                std::to_string(incomplete.percentProgress) + ",";
+                std::to_string(incomplete.percentProgress()) + ",";
         }
         result.pop_back();
     }
@@ -95,15 +95,19 @@ ShardInfo::makeMessage(Application& app)
             tmIncomplete->set_shardindex(shardIndex);
             s.add32(shardIndex);
 
-            auto const state{static_cast<std::uint32_t>(incomplete.state)};
+            static_assert(std::is_same_v<
+                          std::underlying_type_t<decltype(incomplete.state())>,
+                          std::uint32_t>);
+            auto const state{static_cast<std::uint32_t>(incomplete.state())};
             tmIncomplete->set_state(state);
             s.add32(state);
 
             // Set progress if greater than zero
-            if (incomplete.percentProgress > 0)
+            auto const percentProgress{incomplete.percentProgress()};
+            if (percentProgress > 0)
             {
-                tmIncomplete->set_progress(incomplete.percentProgress);
-                s.add32(incomplete.percentProgress);
+                tmIncomplete->set_progress(percentProgress);
+                s.add32(percentProgress);
             }
         }
     }

diff --git a/src/test/nodestore/DatabaseShard_test.cpp b/src/test/nodestore/DatabaseShard_test.cpp
@@ -425,7 +425,7 @@ class DatabaseShard_test : public TestBase
         }
 
         RangeSet<std::uint32_t> rs;
-        from_string(rs, set);
+        BEAST_EXPECT(from_string(rs, set));
         return ripple::to_string(rs);
     }
 
@@ -788,14 +788,14 @@ class DatabaseShard_test : public TestBase
             if (!BEAST_EXPECT(data.makeLedgers(env)))
                 return;
 
+            BEAST_EXPECT(!db->importShard(1, importPath / "not_exist"));
             db->prepareShards({1});
             BEAST_EXPECT(db->getPreShards() == "1");
 
             using namespace boost::filesystem;
             remove_all(importPath / LgrDBName);
             remove_all(importPath / TxDBName);
 
-            BEAST_EXPECT(!db->importShard(1, importPath / "not_exist"));
             if (!BEAST_EXPECT(db->importShard(1, importPath)))
                 return;