Commit d8bbe46eaa6386ac3900a8d75ce7621889bca1f6
1 parent
8cb89529
Update README-maintainer section on use of Member pattern
Showing
1 changed file
with
20 additions
and
10 deletions
README-maintainer.md
| @@ -235,16 +235,26 @@ Building docs from pull requests is also enabled. | @@ -235,16 +235,26 @@ Building docs from pull requests is also enabled. | ||
| 235 | // README-maintainer | 235 | // README-maintainer |
| 236 | ``` | 236 | ``` |
| 237 | 237 | ||
| 238 | -* Put private member variables in std::shared_ptr<Members> for all | ||
| 239 | - public classes. Remember to use QPDF_DLL on ~Members(). Exception: | ||
| 240 | - indirection through std::shared_ptr<Members> is expensive, so don't | ||
| 241 | - do it for classes that are copied a lot, like QPDFObjectHandle and | ||
| 242 | - QPDFObject. It may be possible to declare | ||
| 243 | - std::shared_ptr<Members> m_ph; | ||
| 244 | - Member* m; | ||
| 245 | - with m = m_ph.get(), and then indirect through m in | ||
| 246 | - performance-critical settings, though in 2022, std::shared_ptr is | ||
| 247 | - sufficiently performant that this may not be worth it. | 238 | +* Put private member variables in std::unique_ptr<Members> for all |
| 239 | + public classes. Forward declare Members in the header file and define | ||
| 240 | + Members in the implementation file. One of the major benefits of | ||
| 241 | + defining Members in the implementation file is that it makes it easier | ||
| 242 | + to use private classes as data members and simplifies the include order. | ||
| 243 | + Remember that Members must be fully defined before the destructor of the | ||
| 244 | + main class. For an example of this pattern see class JSONHandler. | ||
| 245 | + | ||
| 246 | + Exception: indirection through std::unique_ptr<Members> incurs an overhead, | ||
| 247 | + so don't do it for: | ||
| 248 | + * (especially private) classes that are copied a lot, like QPDFObjectHandle | ||
| 249 | + and QPDFObject. | ||
| 250 | + * classes that are a shared pointer to another class, such as QPDFObjectHandle | ||
| 251 | + or JSON. | ||
| 252 | + | ||
| 253 | + For exported classes that do not use the member pattern for performance | ||
| 254 | + reasons it is worth considering adding a std::unique_ptr to an empty Members | ||
| 255 | + class initialized to nullptr to give the flexibility to add data members | ||
| 256 | + without breaking the ABI. | ||
| 257 | + | ||
| 248 | 258 | ||
| 249 | * Traversal of objects is expensive. It's worth adding some complexity | 259 | * Traversal of objects is expensive. It's worth adding some complexity |
| 250 | to avoid needless traversals of objects. | 260 | to avoid needless traversals of objects. |