Fix the sonarcloud issues and address Hannah's comments.

Signed-off-by: Johannes Kalmbach <[email protected]>

src/engine/QueryExecutionContext.h

@@ -92,8 +92,8 @@ class QueryExecutionContext {
   [[nodiscard]] const Index& getIndex() const { return _index; }
 
   const LocatedTriplesSnapshot& locatedTriplesSnapshot() const {
-    AD_CORRECTNESS_CHECK(sharedLocatedTriplesSnapshot != nullptr);
-    return *sharedLocatedTriplesSnapshot;
+    AD_CORRECTNESS_CHECK(sharedLocatedTriplesSnapshot_ != nullptr);
+    return *sharedLocatedTriplesSnapshot_;
   }
 
   void clearCacheUnpinnedOnly() { getQueryTreeCache().clearUnpinnedOnly(); }
@@ -130,7 +130,7 @@ class QueryExecutionContext {
   // snapshot of the current (located) delta triples. These can then be used
   // by the respective query without interfering with further incoming
   // update operations.
-  SharedLocatedTriplesSnapshot sharedLocatedTriplesSnapshot{
+  SharedLocatedTriplesSnapshot sharedLocatedTriplesSnapshot_{
       _index.deltaTriplesManager().getCurrentSnapshot()};
   QueryResultCache* const _subtreeCache;
   // allocators are copied but hold shared state

src/index/DeltaTriples.cpp

@@ -196,7 +196,8 @@ DeltaTriplesManager::DeltaTriplesManager(const IndexImpl& index)
       currentLocatedTriplesSnapshot_{deltaTriples_.rlock()->getSnapshot()} {}
 
 // _____________________________________________________________________________
-void DeltaTriplesManager::modify(std::function<void(DeltaTriples&)> function) {
+void DeltaTriplesManager::modify(
+    const std::function<void(DeltaTriples&)>& function) {
   // While holding the lock for the underlying `DeltaTriples`, perform the
   // actual `function` (typically some combination of insert and delete
   // operations) and (while still holding the lock) update the

src/index/DeltaTriples.h

@@ -198,7 +198,7 @@ class DeltaTriplesManager {
   // the current snapshot. Concurrent calls to `modify` will be serialized, and
   // each call to `getCurrentSnapshot` will either return the snapshot before or
   // after a modification, but never one of an ongoing modification.
-  void modify(std::function<void(DeltaTriples&)> function);
+  void modify(const std::function<void(DeltaTriples&)>& function);
 
   // Return a shared pointer to a deep copy of the current snapshot. This can
   // be safely used to execute a query without interfering with future updates.

src/index/LocatedTriples.cpp

@@ -285,7 +285,7 @@ std::ostream& operator<<(std::ostream& os, const std::vector<IdTriple<0>>& v) {
 }
 
 // ____________________________________________________________________________
-bool LocatedTriplesPerBlock::containsTriple(const IdTriple<0> triple,
+bool LocatedTriplesPerBlock::containsTriple(const IdTriple<0>& triple,
                                             bool shouldExist) const {
   auto blockContains = [&triple, shouldExist](const LocatedTriples& lt,
                                               size_t blockIndex) {

src/index/LocatedTriples.h

@@ -169,7 +169,7 @@ class LocatedTriplesPerBlock {
 
   // Only used for testing. Return `true` iff a `LocatedTriple` with the given
   // value for `shouldExist` is contained in any block.
-  bool containsTriple(const IdTriple<0> triple, bool shouldExist) const;
+  bool containsTriple(const IdTriple<0>& triple, bool shouldExist) const;
 
   // This operator is only for debugging and testing. It returns a
   // human-readable representation.

test/DeltaTriplesTest.cpp

@@ -382,12 +382,14 @@ TEST_F(DeltaTriplesTest, DeltaTriplesManager) {
           // Check for several of the thread-exclusive triples that they are
           // properly contained in the current snapshot.
           //
-          // TODO(Hannah): I don't understand the `false` for the second
-          // `containsTriple`.
           auto p = deltaTriplesManager.getCurrentSnapshot();
           const auto& locatedSPO =
               p->getLocatedTriplesForPermutation(Permutation::SPO);
           EXPECT_TRUE(locatedSPO.containsTriple(triplesToInsert.at(1), true));
+          // This triple is exclusive to the thread and is inserted and then
+          // immediately deleted again. The `DeltaTriples` thus only store it as
+          // deleted. It might be contained in the original input, hence we
+          // cannot simply drop it.
           EXPECT_TRUE(locatedSPO.containsTriple(triplesToInsert.at(2), false));
           EXPECT_TRUE(locatedSPO.containsTriple(triplesToDelete.at(2), false));
         }
@@ -409,11 +411,12 @@ TEST_F(DeltaTriplesTest, DeltaTriplesManager) {
 
   // Each of the threads above inserts on thread-exclusive triple, deletes one
   // thread-exclusive triple and inserts one thread-exclusive triple that is
-  // deleted right after. Additionally, there is one common triple inserted by
-  // all the threads and one common triple that is deleted by all the threads.
+  // deleted right after (This triple is stored as deleted in the `DeltaTriples`
+  // because it might be contained in the original input). Additionally, there
+  // is one common triple inserted by// all the threads and one common triple
+  // that is deleted by all the threads.
   //
-  // TODO(Hannah): I don't understand why the number of thread-exclusive deleted
-  // triples is twice that of the thread-exclusive inserted triples.
+
   auto deltaImpl = deltaTriplesManager.deltaTriples_.rlock();
   EXPECT_THAT(*deltaImpl, NumTriples(numThreads + 1, 2 * numThreads + 1,
                                      3 * numThreads + 2));