From 73139a5bda0176b2a7dc62107b0441c089730bd0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 22 Sep 2018 20:43:53 +0200 Subject: [PATCH 1/5] 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. Refs: https://github.com/nodejs/node/pull/23028 Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca Reviewed-By: Joyee Cheung Reviewed-By: Gireesh Punathil Reviewed-By: Daniel Bevenius Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Michael Dawson --- CPP_STYLE_GUIDE.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 14776779fddede..948b7b5272aaf1 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -18,6 +18,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) * [Do not include `*.h` if `*-inl.h` has already been included](#do-not-include-h-if--inlh-has-already-been-included) @@ -200,6 +201,11 @@ void FooConsumer(std::unique_ptr ptr); Never use `std::auto_ptr`. Instead, use `std::unique_ptr`. +### Avoid non-const references + +Using non-const references often obscures which values are changed by an +assignment. A pointer is almost always a better choice. + ## Others ### Type casting From 8daa18eb3744c93222aae9a9cf9f0af300656144 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 1 Oct 2018 08:29:49 -0400 Subject: [PATCH 2/5] squash! doc: formalize non-const reference usage in C++ style guide --- CPP_STYLE_GUIDE.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 948b7b5272aaf1..0602244070ac98 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -206,6 +206,34 @@ Never use `std::auto_ptr`. Instead, use `std::unique_ptr`. Using non-const references often obscures which values are changed by an assignment. A pointer is almost always a better choice. +```c++ +class ExampleClass { + public: + explicit ExampleClass(int* int_ptr) : pointer_to_integer_(int_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_; + int* pointer_to_integer_; // Pointer instead of reference. +}; +``` + ## Others ### Type casting From e9f84067b623a056faa8ddb43290754450df1b09 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 1 Oct 2018 09:09:27 -0400 Subject: [PATCH 3/5] fixup! squash! doc: formalize non-const reference usage in C++ style guide --- CPP_STYLE_GUIDE.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 0602244070ac98..24c45108398654 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -209,7 +209,7 @@ assignment. A pointer is almost always a better choice. ```c++ class ExampleClass { public: - explicit ExampleClass(int* int_ptr) : pointer_to_integer_(int_ptr) {} + 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 @@ -230,7 +230,10 @@ class ExampleClass { private: std::string foo_string_; - int* pointer_to_integer_; // Pointer instead of reference. + // Pointer instead of reference. If this objects 'owns' the other object, + // this should be be a `std::unique_ptr`; a + // `std::shared_ptr` can also be a better choice. + OtherClass* pointer_to_other_; }; ``` From acba30df49557b5dc101b49b1d9f65703508d7ca Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 2 Oct 2018 06:56:18 -0700 Subject: [PATCH 4/5] fixup! fixup! squash! doc: formalize non-const reference usage in C++ style guide --- CPP_STYLE_GUIDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 24c45108398654..569f8809f0106e 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -230,8 +230,8 @@ class ExampleClass { private: std::string foo_string_; - // Pointer instead of reference. If this objects 'owns' the other object, - // this should be be a `std::unique_ptr`; a + // Pointer instead of reference. If this object 'owns' the other object, + // this should be a `std::unique_ptr`; a // `std::shared_ptr` can also be a better choice. OtherClass* pointer_to_other_; }; From 04f941ac965cd638c4dad8baf59ee539759975c3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 18 Oct 2018 23:15:56 +0200 Subject: [PATCH 5/5] fixup! fixup! fixup! squash! doc: formalize non-const reference usage in C++ style guide --- CPP_STYLE_GUIDE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 569f8809f0106e..022951ba85db0b 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -204,7 +204,8 @@ Never use `std::auto_ptr`. Instead, use `std::unique_ptr`. ### Avoid non-const references Using non-const references often obscures which values are changed by an -assignment. A pointer is almost always a better choice. +assignment. Consider using a pointer instead, which requires more explicit +syntax to indicate that modifications take place. ```c++ class ExampleClass {