----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39222/#review102276 -----------------------------------------------------------
proton-c/bindings/cpp/include/proton/connection.hpp (line 71) <https://reviews.apache.org/r/39222/#comment159854> I agree addr and handler are special and deserve to be direct arguments, but I think "dynamic" should be just another link option. It's not the most common use case. proton-c/bindings/cpp/include/proton/link_options.hpp (line 45) <https://reviews.apache.org/r/39222/#comment159853> using `link_option*` introduces a memory management headache. I'd prefer options to be copyable value types. There's a big C++11/C++03 issue here. C++11 has vector initializers and function closures which would make all this much neater, but this is pretty basic functionality to make available only in C++11. Not sure what the best solution is. link_options should itself be public : link_option. That gives you more flexiblity about composing them - you could create a couple of "canned" link_options that set common things for your app, and then add them to a link_options with specific settings for a particular link. proton-c/bindings/cpp/src/link_options.cpp (line 27) <https://reviews.apache.org/r/39222/#comment159857> In C++11 we should support the use of vector initializers. In C++03 a chaining op() would avoid the arbitrary "only 4" restriction and multiple constructors: link_options& link_options::operator()(const link_option &l) { push_back(&l); return *this } // User code foo.create_sender(..., link_options(opt0)(opt1)(opt2)(opt3)(opt4)) The internals are all good but I think we need to work on the user interface - we need to have an example of use so we can see exactly how it looks. I think this design means you can't create option objects as temporaries and pass them in-line, so it looks like: opt1 x(value1) opt2 y(value2) link_options opts(opt1, opt2) create_sender( - Alan Conway On Oct. 12, 2015, 6:56 a.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39222/ > ----------------------------------------------------------- > > (Updated Oct. 12, 2015, 6:56 a.m.) > > > Review request for qpid, Alan Conway, Andrew Stitcher, and Justin Ross. > > > Bugs: PROTON-865 > https://issues.apache.org/jira/browse/PROTON-865 > > > Repository: qpid-proton-git > > > Description > ------- > > Link options, provided much like the Python client. > > Includes examples from Python for selector and browser. > > Additional link and terminus properties were added as required. > > > Diffs > ----- > > examples/cpp/CMakeLists.txt 34edb83 > examples/cpp/queue_browser.cpp PRE-CREATION > examples/cpp/selected_recv.cpp PRE-CREATION > proton-c/bindings/cpp/CMakeLists.txt 45e47b2 > proton-c/bindings/cpp/include/proton/connection.hpp 08b6ad9 > proton-c/bindings/cpp/include/proton/link.hpp 88b5368 > proton-c/bindings/cpp/include/proton/link_options.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/terminus.hpp 4bb9651 > proton-c/bindings/cpp/src/connection.cpp 84c79a6 > proton-c/bindings/cpp/src/link.cpp 12c8790 > proton-c/bindings/cpp/src/link_options.cpp PRE-CREATION > proton-c/bindings/cpp/src/terminus.cpp 2c960d6 > > Diff: https://reviews.apache.org/r/39222/diff/ > > > Testing > ------- > > Linux. Checked attach frames compared to Python. > > > Thanks, > > Cliff Jansen > >
