From 5550510056570ed1266c4c3f9bab07f00d4e8ef2 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 22 Sep 2018 20:43:53 +0200
Subject: doc: formalize non-const reference usage in C++ style guide
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We generally avoid using non-const references if not necessary. This
formalizes this rules by writing them down in the C++ style guide.

(Note: Some reviews are from the original PR.)

Refs: https://github.com/nodejs/node/pull/23028
PR-URL: https://github.com/nodejs/node/pull/23155
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
---
 CPP_STYLE_GUIDE.md | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

(limited to 'CPP_STYLE_GUIDE.md')

diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md
index 36c3538f41..e14de6cc48 100644
--- a/CPP_STYLE_GUIDE.md
+++ b/CPP_STYLE_GUIDE.md
@@ -19,6 +19,7 @@
   * [Memory allocation](#memory-allocation)
   * [Use `nullptr` instead of `NULL` or `0`](#use-nullptr-instead-of-null-or-0)
   * [Ownership and Smart Pointers](#ownership-and-smart-pointers)
+  * [Avoid non-const references](#avoid-non-const-references)
 * [Others](#others)
   * [Type casting](#type-casting)
   * [Using `auto`](#using-auto)
@@ -212,6 +213,43 @@ ownership to the callee and invalidates the caller's instance.
 
 Don't use `std::auto_ptr`, it is deprecated ([Reference][cppref_auto_ptr]).
 
+### Avoid non-const references
+
+Using non-const references often obscures which values are changed by an
+assignment. Consider using a pointer instead, which requires more explicit
+syntax to indicate that modifications take place.
+
+```c++
+class ExampleClass {
+ public:
+  explicit ExampleClass(OtherClass* other_ptr) : pointer_to_other_(other_ptr) {}
+
+  void SomeMethod(const std::string& input_param,
+                  std::string* in_out_param);  // Pointer instead of reference
+
+  const std::string& get_foo() const { return foo_string_; }
+  void set_foo(const std::string& new_value) { foo_string_ = new_value; }
+
+  void ReplaceCharacterInFoo(char from, char to) {
+    // A non-const reference is okay here, because the method name already tells
+    // users that this modifies 'foo_string_' -- if that is not the case,
+    // it can still be better to use an indexed for loop, or leave appropriate
+    // comments.
+    for (char& character : foo_string_) {
+      if (character == from)
+        character = to;
+    }
+  }
+
+ private:
+  std::string foo_string_;
+  // Pointer instead of reference. If this object 'owns' the other object,
+  // this should be a `std::unique_ptr<OtherClass>`; a
+  // `std::shared_ptr<OtherClass>` can also be a better choice.
+  OtherClass* pointer_to_other_;
+};
+```
+
 ## Others
 
 ### Type casting
-- 
cgit v1.2.3