From e5964736d102a5a2ce05baa998b23bbae6745d9c Mon Sep 17 00:00:00 2001
From: Frank Dellaert <dellaert@gmail.com>
Date: Wed, 25 Jan 2023 08:23:12 -0800
Subject: [PATCH] Resolve review comments

---
 gtsam/linear/VectorValues.cpp                 | 29 ++++++++-----------
 .../linear/tests/testLinearEquality.cpp       |  9 ++----
 2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/gtsam/linear/VectorValues.cpp b/gtsam/linear/VectorValues.cpp
index e95e2e38d..cfa0ed9a2 100644
--- a/gtsam/linear/VectorValues.cpp
+++ b/gtsam/linear/VectorValues.cpp
@@ -38,12 +38,8 @@ namespace gtsam {
 
   /* ************************************************************************ */
   VectorValues::VectorValues(const Vector& x, const Dims& dims) {
-    using Pair = pair<const Key, size_t>;
     size_t j = 0;
-    for (const Pair& v : dims) {
-      Key key;
-      size_t n;
-      std::tie(key, n) = v;
+    for (const auto& [key,n] : dims)  {
 #ifdef TBB_GREATER_EQUAL_2020
       values_.emplace(key, x.segment(j, n));
 #else
@@ -70,11 +66,11 @@ namespace gtsam {
   VectorValues VectorValues::Zero(const VectorValues& other)
   {
     VectorValues result;
-    for(const KeyValuePair& v: other)
+    for(const auto& [key,value]: other)
 #ifdef TBB_GREATER_EQUAL_2020
-      result.values_.emplace(v.first, Vector::Zero(v.second.size()));
+      result.values_.emplace(key, Vector::Zero(value.size()));
 #else
-      result.values_.insert(std::make_pair(v.first, Vector::Zero(v.second.size())));
+      result.values_.insert(std::make_pair(key, Vector::Zero(value.size())));
 #endif
     return result;
   }
@@ -92,18 +88,18 @@ namespace gtsam {
   /* ************************************************************************ */
   VectorValues& VectorValues::update(const VectorValues& values) {
     iterator hint = begin();
-    for (const KeyValuePair& key_value : values) {
+    for (const auto& [key,value] : values) {
       // Use this trick to find the value using a hint, since we are inserting
       // from another sorted map
       size_t oldSize = values_.size();
-      hint = values_.insert(hint, key_value);
+      hint = values_.emplace_hint(hint, key, value);
       if (values_.size() > oldSize) {
         values_.unsafe_erase(hint);
         throw out_of_range(
             "Requested to update a VectorValues with another VectorValues that "
             "contains keys not present in the first.");
       } else {
-        hint->second = key_value.second;
+        hint->second = value;
       }
     }
     return *this;
@@ -133,16 +129,15 @@ namespace gtsam {
     // Change print depending on whether we are using TBB
 #ifdef GTSAM_USE_TBB
     map<Key, Vector> sorted;
-    for (const auto& key_value : v) {
-      sorted.emplace(key_value.first, key_value.second);
+    for (const auto& [key,value] : v) {
+      sorted.emplace(key, value);
     }
-    for (const auto& key_value : sorted)
+    for (const auto& [key,value] : sorted)
 #else
-    for (const auto& key_value : v)
+    for (const auto& [key,value] : v)
 #endif
     {
-      os << "  " << StreamedKey(key_value.first) << ": " << key_value.second.transpose()
-         << "\n";
+      os << "  " << StreamedKey(key) << ": " << value.transpose() << "\n";
     }
     return os;
   }
diff --git a/gtsam_unstable/linear/tests/testLinearEquality.cpp b/gtsam_unstable/linear/tests/testLinearEquality.cpp
index 2cd6e8ffd..2f4d41ced 100644
--- a/gtsam_unstable/linear/tests/testLinearEquality.cpp
+++ b/gtsam_unstable/linear/tests/testLinearEquality.cpp
@@ -45,8 +45,7 @@ TEST(LinearEquality, constructors_and_accessors) {
   // Test for using different numbers of terms
   {
     // One term constructor
-    LinearEquality expected(
-        std::vector(terms.begin(), terms.begin() + 1), b, 0);
+    LinearEquality expected(terms.begin(), terms.begin() + 1, b, 0);
     LinearEquality actual(terms[0].first, terms[0].second, b, 0);
     EXPECT(assert_equal(expected, actual));
     LONGS_EQUAL((long)terms[0].first, (long)actual.keys().back());
@@ -57,8 +56,7 @@ TEST(LinearEquality, constructors_and_accessors) {
   }
   {
     // Two term constructor
-    LinearEquality expected(
-        std::vector(terms.begin(), terms.begin() + 2), b, 0);
+    LinearEquality expected(terms.begin(), terms.begin() + 2, b, 0);
     LinearEquality actual(terms[0].first, terms[0].second, terms[1].first,
                           terms[1].second, b, 0);
     EXPECT(assert_equal(expected, actual));
@@ -70,8 +68,7 @@ TEST(LinearEquality, constructors_and_accessors) {
   }
   {
     // Three term constructor
-    LinearEquality expected(
-        std::vector(terms.begin(), terms.begin() + 3), b, 0);
+    LinearEquality expected(terms.begin(), terms.begin() + 3, b, 0);
     LinearEquality actual(terms[0].first, terms[0].second, terms[1].first,
                           terms[1].second, terms[2].first, terms[2].second, b,
                           0);