Clean up warnings in leveldb_transaction.cc (#1711)

* absl::string_view should be passed by value
  * use absl::StrCat and friends for concatenation
  * use C++11 range-based for loops
  * avoid sign conversion

Author: wilhuff

Firestore/core/src/firebase/firestore/local/leveldb_transaction.cc

@@ -20,6 +20,7 @@
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
 #include "Firestore/core/src/firebase/firestore/util/log.h"
 #include "absl/memory/memory.h"
+#include "absl/strings/str_cat.h"
 #include "leveldb/write_batch.h"
 
 using leveldb::DB;
@@ -161,8 +162,7 @@ const WriteOptions& LevelDbTransaction::DefaultWriteOptions() {
   return options;
 }
 
-void LevelDbTransaction::Put(const absl::string_view& key,
-                             const absl::string_view& value) {
+void LevelDbTransaction::Put(absl::string_view key, absl::string_view value) {
   std::string key_string(key);
   std::string value_string(value);
   mutations_[key_string] = value_string;
@@ -175,13 +175,12 @@ LevelDbTransaction::NewIterator() {
   return absl::make_unique<LevelDbTransaction::Iterator>(this);
 }
 
-Status LevelDbTransaction::Get(const absl::string_view& key,
-                               std::string* value) {
+Status LevelDbTransaction::Get(absl::string_view key, std::string* value) {
   std::string key_string(key);
   if (deletions_.find(key_string) != deletions_.end()) {
     return Status::NotFound(key_string + " is not present in the transaction");
   } else {
-    Mutations::iterator iter(mutations_.find(key_string));
+    Mutations::iterator iter{mutations_.find(key_string)};
     if (iter != mutations_.end()) {
       *value = iter->second;
       return Status::OK();
@@ -191,7 +190,7 @@ Status LevelDbTransaction::Get(const absl::string_view& key,
   }
 }
 
-void LevelDbTransaction::Delete(const absl::string_view& key) {
+void LevelDbTransaction::Delete(absl::string_view key) {
   std::string to_delete(key);
   deletions_.insert(to_delete);
   mutations_.erase(to_delete);
@@ -200,12 +199,12 @@ void LevelDbTransaction::Delete(const absl::string_view& key) {
 
 void LevelDbTransaction::Commit() {
   WriteBatch batch;
-  for (auto it = deletions_.begin(); it != deletions_.end(); it++) {
-    batch.Delete(*it);
+  for (const auto& deletion : deletions_) {
+    batch.Delete(deletion);
   }
 
-  for (auto it = mutations_.begin(); it != mutations_.end(); it++) {
-    batch.Put(it->first, it->second);
+  for (const auto& entry : mutations_) {
+    batch.Put(entry.first, entry.second);
   }
 
   LOG_DEBUG("Committing transaction: %s", ToString());
@@ -216,21 +215,21 @@ void LevelDbTransaction::Commit() {
 }
 
 std::string LevelDbTransaction::ToString() {
-  std::string dest("<LevelDbTransaction " + label_ + ": ");
-  int64_t changes = deletions_.size() + mutations_.size();
-  int64_t bytes = 0;  // accumulator for size of individual mutations.
+  std::string dest = absl::StrCat("<LevelDbTransaction ", label_, ": ");
+  size_t changes = deletions_.size() + mutations_.size();
+  size_t bytes = 0;  // accumulator for size of individual mutations.
   dest += std::to_string(changes) + " changes ";
   std::string items;  // accumulator for individual changes.
-  for (auto it = deletions_.begin(); it != deletions_.end(); it++) {
-    items += "\n  - Delete " + DescribeKey(*it);
+  for (const auto& deletion : deletions_) {
+    absl::StrAppend(&items, "\n  - Delete ", DescribeKey(deletion));
   }
-  for (auto it = mutations_.begin(); it != mutations_.end(); it++) {
-    int64_t change_bytes = it->second.length();
+  for (const auto& entry : mutations_) {
+    size_t change_bytes = entry.second.size();
     bytes += change_bytes;
-    items += "\n  - Put " + DescribeKey(it->first) + " (" +
-             std::to_string(change_bytes) + " bytes)";
+    absl::StrAppend(&items, "\n  - Put ", DescribeKey(entry.first), " (",
+                    change_bytes, " bytes)");
   }
-  dest += "(" + std::to_string(bytes) + " bytes):" + items + ">";
+  absl::StrAppend(&dest, "(", bytes, " bytes):", items, ">");
   return dest;
 }
 

Firestore/core/src/firebase/firestore/local/leveldb_transaction.h

@@ -154,14 +154,14 @@ class LevelDbTransaction {
    * Remove the database entry (if any) for "key".  It is not an error if "key"
    * did not exist in the database.
    */
-  void Delete(const absl::string_view& key);
+  void Delete(absl::string_view key);
 
 #if __OBJC__
   /**
    * Schedules the row identified by `key` to be set to the given protocol
    * buffer message when this transaction commits.
    */
-  void Put(const absl::string_view& key, GPBMessage* message) {
+  void Put(absl::string_view key, GPBMessage* message) {
     NSData* data = [message data];
     std::string key_string(key);
     mutations_[key_string] = std::string((const char*)data.bytes, data.length);
@@ -173,15 +173,15 @@ class LevelDbTransaction {
    * Schedules the row identified by `key` to be set to `value` when this
    * transaction commits.
    */
-  void Put(const absl::string_view& key, const absl::string_view& value);
+  void Put(absl::string_view key, absl::string_view value);
 
   /**
    * Sets the contents of `value` to the latest known value for the given key,
    * including any pending mutations and `Status::OK` is returned. If the key
    * doesn't exist in leveldb, or it is scheduled for deletion in this
    * transaction, `Status::NotFound` is returned.
    */
-  leveldb::Status Get(const absl::string_view& key, std::string* value);
+  leveldb::Status Get(absl::string_view key, std::string* value);
 
   /**
    * Returns a new Iterator over the pending changes in this transaction, merged