Github user astitcher commented on the pull request: https://github.com/apache/qpid-proton/pull/35#issuecomment-109383948 Some overall comments (from a not yet thorough) read through: * I really don't like using set/get in a C++ API. It's not idiomatic C++ API style; it's not necessary; and it doesn't read as well (IMO) - So instead of `Proton::transport::setXXXX('blah')`/`Proton::transport::getXXXX()`, can we please use `Proton::transport::XXXX('blah')`/`Proton::transport::XXXX()`. We use idiomatic properties in Python we should use them in C++ too. * I'm not sure that we really need to use the Pimpl pattern for this binding. The real need for a Pimpl is to insulate this API from changes to code that it depends on - in this case once the C API is stable then its own API stability requirements should ensure that nothing gets broken in the C++ API too. I may not have thought this through hard enough though [is that a sentence with too many 'ough's or what ;-) ] This will save the indirection and extra resource use that Pimpl brings, which would force 2 allocations for each of the structures. This strikes me as important because the C++ API can be as efficient as the C API if we do this work well, and it will be much easier to write and read code in good C++ than C. So I think that no one should need to do any application work in C even for applications that need the full efficiency of low level control of the library.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---