From 1a88c5a5379dbdc4b9873fef3cdeb2405614fb12 Mon Sep 17 00:00:00 2001 From: beinhaerter <34543625+beinhaerter@users.noreply.github.com> Date: Wed, 7 Aug 2019 22:28:52 +0200 Subject: [PATCH] make the sample in Sd-factory and C.50 compileable (closes #1205, #1488) (#1489) * make the sample in Sd-factory compileable (closes #1488) - make the sample in Sd-factory compileable - fixed wrong capitalization: create/Create -> create - `make_shared` cannot access protected constructors, so made them public. To still have access protection introduced a protected `class Token` in each class. That token can only be created by the class itself (and derived classes) and needs to be passed to the constructor. - changed order: `public` first, then `protected` - same sample for C.50 and Sd-factory - removed spurious "see Item 49.1" as it is unclear what this means * line length * tabs -> spaces * spelling * input from cubbimew - added back in Item 49.1 - added link for items as suggested ("in [SuttAlex05](#SuttAlex05)") * changed link to Item 49.1 to link to C.82 --- CppCoreGuidelines.md | 95 ++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 2f843df..acf3b11 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -5620,49 +5620,55 @@ The return type of the factory should normally be `unique_ptr` by default; if so class B { public: - B() - { - // ... - f(); // BAD: virtual call in constructor - // ... + B() { + /* ... */ + f(); // BAD: C.82: Don't call virtual functions in constructors and destructors + /* ... */ } virtual void f() = 0; - - // ... }; ##### Example class B { protected: - B() { /* ... */ } // create an imperfectly initialized object - - virtual void PostInitialize() // to be called right after construction - { - // ... - f(); // GOOD: virtual dispatch is safe - // ... - } + class Token {}; public: + explicit B(Token) { /* ... */ } // create an imperfectly initialized object virtual void f() = 0; template - static shared_ptr Create() // interface for creating shared objects + static shared_ptr create() // interface for creating shared objects { - auto p = make_shared(); - p->PostInitialize(); + auto p = make_shared(typename T::Token{}); + p->post_initialize(); return p; } + + protected: + virtual void post_initialize() // called right after construction + { /* ... */ f(); /* ... */ } // GOOD: virtual dispatch is safe }; - class D : public B { /* ... */ }; // some derived class + class D : public B { // some derived class + protected: + class Token {}; - shared_ptr p = D::Create(); // creating a D object + public: + explicit D(Token) : B{ B::Token{} } {} + void f() override { /* ... */ }; -By making the constructor `protected` we avoid an incompletely constructed object escaping into the wild. -By providing the factory function `Create()`, we make construction (on the free store) convenient. + protected: + template + friend shared_ptr B::create(); + }; + + shared_ptr p = D::create(); // creating a D object + +`make_shared` requires that the constructor is public. By requiring a protected `Token` the constructor cannot be publicly called anymore, so we avoid an incompletely constructed object escaping into the wild. +By providing the factory function `create()`, we make construction (on the free store) convenient. ##### Note @@ -21494,53 +21500,64 @@ Here is an example of the last option: class B { public: - B() { /* ... */ f(); /* ... */ } // BAD: see Item 49.1 + B() { + /* ... */ + f(); // BAD: C.82: Don't call virtual functions in constructors and destructors + /* ... */ + } virtual void f() = 0; - - // ... }; class B { protected: - B() { /* ... */ } - virtual void post_initialize() // called right after construction - { /* ... */ f(); /* ... */ } // GOOD: virtual dispatch is safe + class Token {}; + public: + // constructor needs to be public so that make_shared can access it. + // protected access level is gained by requiring a Token. + explicit B(Token) { /* ... */ } // create an imperfectly initialized object virtual void f() = 0; template - static shared_ptr create() // interface for creating objects + static shared_ptr create() // interface for creating shared objects { - auto p = make_shared(); + auto p = make_shared(typename T::Token{}); p->post_initialize(); return p; } - // ... + protected: + virtual void post_initialize() // called right after construction + { /* ... */ f(); /* ... */ } // GOOD: virtual dispatch is safe + } }; class D : public B { // some derived class + protected: + class Token {}; + public: + // constructor needs to be public so that make_shared can access it. + // protected access level is gained by requiring a Token. + explicit D(Token) : B{ B::Token{} } {} void f() override { /* ... */ }; protected: - D() {} - template - friend shared_ptr B::Create(); + friend shared_ptr B::create(); }; - shared_ptr p = D::Create(); // creating a D object + shared_ptr p = D::create(); // creating a D object This design requires the following discipline: -* Derived classes such as `D` must not expose a public constructor. Otherwise, `D`'s users could create `D` objects that don't invoke `PostInitialize`. -* Allocation is limited to `operator new`. `B` can, however, override `new` (see Items 45 and 46). -* `D` must define a constructor with the same parameters that `B` selected. Defining several overloads of `Create` can assuage this problem, however; and the overloads can even be templated on the argument types. +* Derived classes such as `D` must not expose a publicly callable constructor. Otherwise, `D`'s users could create `D` objects that don't invoke `post_initialize`. +* Allocation is limited to `operator new`. `B` can, however, override `new` (see Items 45 and 46 in [SuttAlex05](#SuttAlex05)). +* `D` must define a constructor with the same parameters that `B` selected. Defining several overloads of `create` can assuage this problem, however; and the overloads can even be templated on the argument types. -If the requirements above are met, the design guarantees that `PostInitialize` has been called for any fully constructed `B`-derived object. `PostInitialize` doesn't need to be virtual; it can, however, invoke virtual functions freely. +If the requirements above are met, the design guarantees that `post_initialize` has been called for any fully constructed `B`-derived object. `post_initialize` doesn't need to be virtual; it can, however, invoke virtual functions freely. In summary, no post-construction technique is perfect. The worst techniques dodge the whole issue by simply asking the caller to invoke the post-constructor manually. Even the best require a different syntax for constructing objects (easy to check at compile time) and/or cooperation from derived class authors (impossible to check at compile time).