Re: [boost] [pre-review] Pimpl submission in the review queue
I have a Pimpl generalization submission getting close to the top in the review queue. Rob Stewart suggested I'd post it to the new Boost Code Collaborator site for a pre-review. That's what I did with the following link to the pre-review (Review #5):
http://demo.smartbear.com/boost/go?page=ReviewDisplay&reviewid=5
Your participation in that pre-review is most welcome.
Fantastic! I signed up and am anxious to see this library accepte^Wreviewed^Waccepted. I've been using the Boost.Pimpl interface for nearly a year now and have been very successful and happy with its use in my projects. Second to ASIO, Boost.Pimpl has done more to impact the design, structure and maintenance of my projects than any other library (the benefits of a stable ABI are understated). Its use in maintaining clean interfaces and projects can't be understated. Historically I'd always saved implementing Pimpl for external facing bits and had always rolled my own interface, now I use it for everything internal or external because of how easy it is to thump out a Boost.Pimpl based interface. As I said on the review site, I've moved the Pimpl implementation in to "boost/pimpl" directory with success, though was not able to move the Pimpl library under the boost namespace. Actually... let me post a quick example that I wrote to both learn and quickly teach how to use Pimpl: https://github.com/sean-/Boost.Examples/tree/master/pimpl It takes about 60sec-3min for most people to have their "ah ha!" moment with that example (those unfamiliar with Pimpl would be wise to check it out, imnsho). I didn't dive in to the why, but in the implementation it is necessary open your namespace after you have defined your private implementation (see pimpl/pimpl_example_pointer_impl.cc:35). It would be nice to know why this is the case (or if it's a bug). Pre-review feedback: The examples used in the Pimpl unit tests are very very good, but it takes a few minutes to wade through the polymorphism and to tease out the most basic requirements for starting to use Pimpl. Walking through 1K of code to do something very simple is a bit daunting despite the obvious and nearly immediate dividends. Whenever I started using this, it was one of the best few hours I've ever spent learning something new. I'm very pleased to see this library move forward and look forward to its formal review. -sc -- Sean Chittenden sean@chittenden.org
Sean Chittenden-4 wrote:
Actually... let me post a quick example that I wrote to both learn and quickly teach how to use Pimpl:
https://github.com/sean-/Boost.Examples/tree/master/pimpl
It takes about 60sec-3min for most people to have their "ah ha!" moment with that example (those unfamiliar with Pimpl would be wise to check it out, imnsho).
I didn't dive in to the why, but in the implementation it is necessary open your namespace after you have defined your private implementation (see pimpl/pimpl_example_pointer_impl.cc:35). It would be nice to know why this is the case (or if it's a bug).
I guess this is because the implementation must be a specialization of the class pimpl, which is not in your name space. An alternative design could be to let you define your implementation class and use a trait to select the implementation. namespace org { namespace example { struct MyStringImpl {...}; } // namespace example } // namespace org template <> struct pimpl_trait<org::example::String> { typedef org::example::String type; }; Vladimir, what do you think? Is the use of (*this)-> in your example really needed? bool String::append(unsigned char byte) { (*this)->str_->append(1, byte); return true; } Or does the following work? bool String::append(unsigned char byte) { this->str_->append(1, byte); return true; } BTW, if you want to simplify a little bit your example you could use string str_; instead of string* str; Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/pre-review-Pimpl-submission-in-the-review... Sent from the Boost - Dev mailing list archive at Nabble.com.
I have a question concerning the review tool : is there a place where to write classic reviews (with general feedback about the library) ? It seems that it's mainly oriented to code review, not really general library review. Joël Lamotte
Klaim - Joël Lamotte wrote:
I have a question concerning the review tool : is there a place where to write classic reviews (with general feedback about the library)? It seems that it's mainly oriented to code review, not really general library review.
The tool is definitely geared toward code review. Note, however, that "code" does include documentation sources, so it's a good place to provide documentation comments, too. If you wish to provide higher level comments, you can do that in the General Chat area or here on the list. If you wish to provide a normal review, then the list remains the place to do that. There may yet be a tool out there that can do both effectively, and we haven't decided to adopt Code Collaborator. Others have begun looking at Crucible, but we need to set up real libraries in Crucible to get similarly good experience with it. Please suggest other tools that are free (or inexpensive) for open source projects. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
participants (4)
-
Klaim - Joël Lamotte -
Sean Chittenden -
Stewart, Robert -
Vicente Botet