[Development] Don't use Private *d when your class is immutable

[Lars Knoll]

I agree, that we should not use any implicit sharing in those cases. But in many cases, I don’t see a problem using a pointer to the data. Of course if shouldn’t detach. Instead, it should be explicitly shared (with or without refcounting depending on whether the data is generated at compile or runtime). Using an integer and a global lookup table would lead to more complex code in that case.

Cheers,
Lars

> On 03 Mar 2017, at 08:30, Marc Mutz <marc.mutz at kdab.com> wrote:
> 
> Hi,
> 
> If you have a class that just represents a concrete immutable entity, like 
> some system resource's ID, or an SSL elliptic curve algorithm, you can use an 
> enum.
> 
> Or you can use a class, if you want some richer API than can be had with an 
> enum.
> 
> In that case, however, *don't* use a Private pointer.
> 
> The class is immutable, there's no setters, so why should the private parts be 
> implicitly shared? They are naturally globally shared, so just use an int as a 
> member and let the compiler deal with the implementaton of the copy and move 
> special member functions. The property getters just use the int as an index 
> into a global data structure to retrieve the property values.
> 
> Good example: QSslEllipticCurve. The int is an index into OpenSSL's internal 
> list of elliptic curves, and an int is by what OpenSSL refers to an elliptic 
> curve. Any other backend will either have a library that does the same or 
> needs to provide some central array/vector with the data. Don't be lazy here. 
> The internals are more complicated, but the type is just an int to the 
> compiler. The API is more important than the implementation.
> 
> Bad example: QSslCipher. Look at all the messy API that just deals with the 
> fact it's pimpled. That class is particularly hideous because it allocates 
> memory on every copy!
> 
> For fun, I de-pimpled a private class. It saved 2+KiB in text size: 
> https://codereview.qt-project.org/149499 So to the best of our knowledge, a 
> pimpl costs O(KiB) of executable size.
> 
> Pimpl is not a virtue, it's a burden. Don't use it unless you must. A 
> necessary condition is when there are more than a small, fixed number of 
> states in your class. This condition is not sufficient, however, as witnessed 
> by QString.
> 
> So, please, when reviewing API (now in the final reviews for 5.9 as well as 
> when adding new API), don't accept classes that end in Info or Id and contain 
> a d-pointer.
> 
> Thanks,
> Marc
> 
> -- 
> Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
> KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
> Tel: +49-30-521325470
> KDAB - The Qt, C++ and OpenGL Experts
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development