diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 14776779fddede..022951ba85db0b 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,43 @@ 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. 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`; a + // `std::shared_ptr` can also be a better choice. + OtherClass* pointer_to_other_; +}; +``` + ## Others ### Type casting