Artyom Beilis <artyomtnk <at> yahoo.com> writes: ... I glad you do not review KDE's and Qt's code
I am glad I do not have to. I've seen it. It's not exactly pretty. It's somewhat understandable as their code base has evolved over quite considerable period. Say, their signal/slot idea is so much better implemented using boost::signal rather than using some obscure macros, qt-precompiler, run-time detection of incorrect signatures. It does not make the solutions they had to resort to common practice though.
See:
http://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++#Using_... Pointer
This is common practice.
From that URL: The d-pointer pattern has been described many times in computer science history under various names, e.g. as pimpl, as handle/body or as cheshire cat.
So, as you can see Pimpl *is* a d-pointer. It beats me why it's so important to support another d-pointer inside Pimpl (which by author's description is a d-pointer).
The library addresses a certain well-defined concept/issue. It is about constructing a Pimpl class easily, safely and painlessly.
It is ok, but all I'm trying to say (may be in 5th time) that pimpl is little bit more then that.
It is ok that Boost would have the "pimpl" template you suggest but it should not be "Pimpl" library but a part of pimpl library.
Pimpl - is much wider topic (IMHO).
I disagree that Pimpl is "much wider topic". The idiom is well-described and well-defined in various sources. I think in the original article I mentioned sources which I based my understanding of Pimpl on. I believe I was faithful to the last letter to those sources and to the Pimpl idiom definition. Could you please list the sources where you get your extended interpretation of the Pimpl idiom from? Pimpl is essentially another smart pointer and all of them (shared_ptr, unique_ptr, etc.) have their well-defined qualities, properties and applications. If we start arbitrarily "extending" those definitions and their applications beyond what those pointers were designed for, we'll end up with a mess.
... I had shown you 2-3 examples of such cases where it is not suitable but
I'll repeat:
a) Wrapping different implementation (not mine) with different interface
class ustring { private: smart_ptr<icu::UnicodeString> pimpl_; };
With my Pimpl it's done as follows: // Interface struct ustring : pimpl<ustring>::whatever { pure interface; }; // Implementation template<> pimpl<ustring>::implementation { icu::UnicodeString data; } or template<> pimpl<ustring>::implementation : public icu::UnicodeString { }
b) Improving compilation speed by hiding a real object behind a lighweight wrapper.
// // Real implementation uses havy classes in headers like Asio. // used in many locations, // class connection { public: ... private: boost::asio::ip::tcp::socket socket_; ... };
// // Lighweight wrapper for basic functionality that is // used in places where I don't want to include full ASIO. // to do very simple stuff where connection is the real // fully functional object. // class connector { public: ... void setip(std::string ...) connection &impl(); ... private: smart_ptr<connection> pimpl_; };
With my Pimpl it's done as follows: // Interface struct connector : pimpl<connector>::whatever { pure interface; }; // Implementation template<> pimpl<connector>::implementation { connection data; } or template<> pimpl<connector>::implementation : public connection { }
c) Not calling class "implementation" just because it only holds data memebers and not real implementations (AKA d-pointer)
Only to make adding new members not to change its sizeof
struct book { private: // data members - not functions. smart_ptr<data> d; };
So having pimpl<book>::implementation is misleading as it is only... discuonnected part of body - have data members.
Applying Pimpl idiom to the book above is, uhm... err, ...makes no sense to me. I'd suggest // Interface struct book : pimpl<book>::whatever { interface; } // Implementation template<> pimpl<book>::implementation : public data {}
Any more arguments?
No, I am just tired. Let's just agree to disagree.
Consider this case:
statistics.h
class statistics { public: ... void add_new_value(double v); double std() const; double mean() const; ... private: // enough to handle standard deviation and mean values int N_; double sum_; double square_sum_;
// Place holder for future use // for statitics that can't be calculated with // only the values above. struct data; smart_ptr<data> d; };
statistics.cpp .. void statistics::add_new_value(double v) { N_++; sum_+=v; square_sum_ += v*v; } ...
In this case you keep a smart pointer just as place holder not in use, the class is generally simple value type but you want to keep it extensible.
This class does not even require memory allocation (at this point) But once when a new type of statistics is added the "d" pointer would be in use.
Is this valid use case or not?
Yes, it is a valid use-case... poorly implemented. I'd apply normal Pimpl idiom, i.e. I'd separate interface from the data/implementation; I'd hide the implementation and that includes int N_; double sum_; double square_sum_; like template<> pimpl<statistics>::implementation { int N_; double sum_; double square_sum_; }; Then I'd extend pimpl<statistics>::implementation when my requirements change. In fact, I already do exactly that. If I want to avoid memory allocation, then, I'd use a policy as Gottlob Frege suggested... but I'd keep design clean and simple.
I'm just trying you to show many other use cases that I had met, used, deployed in my own and not-connected-to-me-at-all software.
You provided a nice pimpl framework but *in my opinoin* it does not cover enough use cases. If a real formal review was running these days, you would get my negative vote.
Yes, I see and understand your position and your needs (they are not really that different from mine -- I face similar issues). We happen to approach things differently. And that's OK. It's equally OK if you vote yes, no, whatever. In all honesty I do not care who and how votes... and I won't write your name into the list of my enemies. If fact, I kinda like you (even though you've been getting on my nerves lately :-) ). Equally I do not really care if Pimpl is accepted into Boost or not. Quite some time back I thought I had something simple and useful for others. So, I was willing to share it with others. If people vote it out, then they do not need it. And that's fine by me. Now I am just tired and want to get it over with one way or the other. And, yes, I admit I've been quite nasty to you lately. My honest and humble apologies. My only excuse could be that "you started it"... but I am a bit old to use that line of reasoning. So, I have no excuses. I am sorry. I wish you all the very best and I wish your pimples soundly beat mine. ;-) V.