Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
On Mon, Sep 10, 2012 at 3:38 PM, Andrew Stitcher wrote: > On Mon, 2012-09-10 at 15:28 -0400, Rafael Schloming wrote: > > On Mon, Sep 10, 2012 at 3:23 PM, Andrew Stitcher >wrote: > > > > > On Mon, 2012-09-10 at 15:01 -0400, Rafael Schloming wrote: > > > > Can you comment on why you decided to inline it directly into the C > > > header > > > > files as opposed to splitting it out somehow? Given William's > comment on > > > > exceptions, it seems like it might well expand/evolve enough to make > it > > > > awkward to inline. > > > > > > This was partly for speed, and partly because I tend to think of C and > C > > > ++ as peer languages so that if you're using proton from C++ you would > > > just use the same include files as using it from C. > > > > > > > Isn't there a way to split out the C++ stuff but still use the same > include > > files, e.g. include the C++ stuff rather than inline it? > > What would that gain? In any case if you mean have an extra #include > inside the #ifdef __cplusplus that could certainly be added if the file > got unmanageably large. > > I guess the question here is really the nature of the relationship between the C++ code and the C library. To date I've been able to remain blissfully ignorant of C++. With this stuff in here if I were to make a change to part of the C API (while still remaining blissfully ignorant of C++) then I could quite possibly break the C++ code, and the C portion of the source file might get ahead of the C++ portion of the source file. This is of course an issue with all the other bindings as well, but to some degree it is mitigated by swig managing to do the right thing much of the time. Notably in a couple cases where it doesn't there has been some breakage caused in a similar manner. I guess the root of the question is really package structure/workflow between development on the bindings and development on the core library. To what degree should we treat the bindings as independent packages that track a specific version of the library and might in fact lag it a little, and to what degree do we treat the whole thing as a unit and try to keep all the bindings 100% up to date at all times. I'm not sure about the answer right now, but having the C++ code inline with the C code seems to rule out (or at least make really awkward) the possibility of the C++ binding lagging the C library. On another note, to offer some commentary on the content of the API as opposed to it's delivery mechanism, I think it is a definitely a good thing to have and I'd like to see it included somehow. I'm not really a C++ expert, so I can't comment too deeply, but the one (ok most likely the first) thing I'd say is that the low overhead wrapping approach might actually be more beneficial with the lower level parts of the API that you haven't yet wrapped, whereas the overhead issues with a more idiomatic C++ wrapping of messenger are likely negligible. --Rafael
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
On Mon, 2012-09-10 at 15:28 -0400, Rafael Schloming wrote: > On Mon, Sep 10, 2012 at 3:23 PM, Andrew Stitcher wrote: > > > On Mon, 2012-09-10 at 15:01 -0400, Rafael Schloming wrote: > > > Can you comment on why you decided to inline it directly into the C > > header > > > files as opposed to splitting it out somehow? Given William's comment on > > > exceptions, it seems like it might well expand/evolve enough to make it > > > awkward to inline. > > > > This was partly for speed, and partly because I tend to think of C and C > > ++ as peer languages so that if you're using proton from C++ you would > > just use the same include files as using it from C. > > > > Isn't there a way to split out the C++ stuff but still use the same include > files, e.g. include the C++ stuff rather than inline it? What would that gain? In any case if you mean have an extra #include inside the #ifdef __cplusplus that could certainly be added if the file got unmanageably large. A
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
On Mon, Sep 10, 2012 at 3:23 PM, Andrew Stitcher wrote: > On Mon, 2012-09-10 at 15:01 -0400, Rafael Schloming wrote: > > Can you comment on why you decided to inline it directly into the C > header > > files as opposed to splitting it out somehow? Given William's comment on > > exceptions, it seems like it might well expand/evolve enough to make it > > awkward to inline. > > This was partly for speed, and partly because I tend to think of C and C > ++ as peer languages so that if you're using proton from C++ you would > just use the same include files as using it from C. > Isn't there a way to split out the C++ stuff but still use the same include files, e.g. include the C++ stuff rather than inline it? --Rafael
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
On Mon, 2012-09-10 at 15:01 -0400, Rafael Schloming wrote: > Can you comment on why you decided to inline it directly into the C header > files as opposed to splitting it out somehow? Given William's comment on > exceptions, it seems like it might well expand/evolve enough to make it > awkward to inline. This was partly for speed, and partly because I tend to think of C and C ++ as peer languages so that if you're using proton from C++ you would just use the same include files as using it from C. I'm minded more to add exceptions on top of this very slim binding so that you could use proton from C++ with no more excess baggage than if you were using C (but with a much nicer syntax). This is especially true of the lower level classes rather than the rather higher level Messenger class. Andrew
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
- Original Message - > On Mon, 2012-09-10 at 14:16 -0400, William Henry wrote: > > It looks very attractive. Any plans to include exceptions? > > I was debating exceptions: On the one hand I wanted a very slim layer > with no hidden code - what is there now will pretty much compile > directly in to the same code as the equivalent C code. On the other > error handling using exceptions is much more idiomatic for a fuller > C++ > interface. Yep. +1 for exceptions. I don't think I'd worry about C++ code fitting in with the C code. > > > > > (I like the use of 'auto' keyword. Had not seen it in code like > > this before.) > > This is C++11 (the new version of the C++ language) which has type > inference built into the language. I wanted to make sure /demonstrate > that the bindings would work nicely with the new version of the > language > (as well as with the old version). > Yea that is cool. I haven't read up on the pros and cons of the feature but it looks cool. I could see some potential debates over maintainability/readability - "you don't know what type it's supposed to be when reading the code." etc. But I guess that's just a matter of preference. William > Andrew > > >
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
Can you comment on why you decided to inline it directly into the C header files as opposed to splitting it out somehow? Given William's comment on exceptions, it seems like it might well expand/evolve enough to make it awkward to inline. --Rafael On Mon, Sep 10, 2012 at 2:47 PM, Andrew Stitcher wrote: > On Mon, 2012-09-10 at 14:16 -0400, William Henry wrote: > > It looks very attractive. Any plans to include exceptions? > > I was debating exceptions: On the one hand I wanted a very slim layer > with no hidden code - what is there now will pretty much compile > directly in to the same code as the equivalent C code. On the other > error handling using exceptions is much more idiomatic for a fuller C++ > interface. > > > > > (I like the use of 'auto' keyword. Had not seen it in code like this > before.) > > This is C++11 (the new version of the C++ language) which has type > inference built into the language. I wanted to make sure /demonstrate > that the bindings would work nicely with the new version of the language > (as well as with the old version). > > Andrew > > >
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
On Mon, 2012-09-10 at 14:16 -0400, William Henry wrote: > It looks very attractive. Any plans to include exceptions? I was debating exceptions: On the one hand I wanted a very slim layer with no hidden code - what is there now will pretty much compile directly in to the same code as the equivalent C code. On the other error handling using exceptions is much more idiomatic for a fuller C++ interface. > > (I like the use of 'auto' keyword. Had not seen it in code like this before.) This is C++11 (the new version of the C++ language) which has type inference built into the language. I wanted to make sure /demonstrate that the bindings would work nicely with the new version of the language (as well as with the old version). Andrew
Re: First cut at a C++ "binding" for proton comments please [Fwd: Review Request: C++ wrappers for proton pn_message_t and pn_messenger_t]
It looks very attractive. Any plans to include exceptions? (I like the use of 'auto' keyword. Had not seen it in code like this before.) William - Original Message - > There is no proton group in reviewboard, so I'm forwarding this here > for > comment. > > Andrew > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6929/ > --- > > Review request for qpid, Gordon Sim, Cliff Jansen, and Rafael > Schloming. > > > Description > --- > > As an experiment I've wrapped the pn_message_t and Pn_messenger_t > structs and operations in very lightweight c++ classes pn::Message > and pn::Messenger. > > To see how this looks I've also transcribed the python messenger > examples to these new C++ wrappers. > > If this is generally liked then I propose to wrap the rest of the > proton API similarly. > > Some possible controversial (and definitely revisable) decisions I > made here: > > * I don't like to prefix setters and getters with "set" and "get" so > I didn't > > * I put the wrappers in the supplied header files rather than in a > new header file. > > > Diffs > - > > /proton/trunk/examples/c++/client.cpp PRE-CREATION > /proton/trunk/examples/c++/recv.cpp PRE-CREATION > /proton/trunk/examples/c++/send.cpp PRE-CREATION > /proton/trunk/examples/c++/server.cpp PRE-CREATION > /proton/trunk/proton-c/include/proton/message.h 1381265 > /proton/trunk/proton-c/include/proton/messenger.h 1381265 > > Diff: https://reviews.apache.org/r/6929/diff/ > > > Testing > --- > > Compiled and ran the transcribed c++ examples > > > Thanks, > > Andrew Stitcher > > > >
