> On Aug. 7, 2013, 7:22 p.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/src/qpid/types/encodings.h, line 26 > > <https://reviews.apache.org/r/13328/diff/2/?file=338357#file338357line26> > > > > I'm not sure a namespace is best here, maybe a struct with statics > > would be better here - in C++11 you can initialise static strings in class > > definition scopes, however here you will need a .cpp file. > > Gordon Sim wrote: > What's the issue with a namespace? > > Andrew Stitcher wrote: > no issue specifically, but I don't think we've introduced a namespace > before simply to contain some constants. I'd usually expect constants to be > in the namespace/class that uses them. In this case just qpid::types::BINARY > etc. However I can see that you want to indicate that they are encodings and > I guess you don't like the wordiness of qpid::types::BINARY_ENCODING > preferring qpid::types::encodings::BINARY. > > My actual preference would be simple qpid::types::BINARY, but it is > largely an aesthetic judgement. given that we have no established convention > in this API. > > Gordon Sim wrote: > I have other examples (qpid/amqp/typecodes.h, qpid/amqp/descriptors.h). I > can move them into the types namespace if you feel strongly enough about it, > but I felt this was clearer and tidier.
I'm not sure what you mean by "other examples" Are those meant to be part of the external messaging API? If they are then they will need to be in it somewhere, if not then I'm not sure they are exactly relevant. There is one way in which your proposal is clearly tidier - you can simply do "using namespace qpid::types::encodings" to import all the definitions. My strong preference is to import each name individually so I don't find that compelling. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13328/#review24805 ----------------------------------------------------------- On Aug. 7, 2013, 7:17 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13328/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2013, 7:17 p.m.) > > > Review request for qpid. > > > Bugs: QPID-5040 > https://issues.apache.org/jira/browse/QPID-5040 > > > Repository: qpid > > > Description > ------- > > In AMQP 1.0, though you can of course still send raw data (as a Data section) > with a content-type specified in the properties, the preferred method of > sending maps and lists is through an AmqpValue section with the type encoded > in the bytes making up that section. > > Over 0-10, the approach used was to have encode/decode functions that work > with a Variant::Map or Variant::List. The application was responsible for > encoding outgoing messages before sending them and checking the content-type > on received messages and decoding them if appropriate. > > Now that the content-type cannot be relied on to indicate the type of the > message (indeed the specification encourages it *not* to be used, relying > only on the type encoding in the AmqpValue section), a different approach is > necessary. > > I've added a method to Message to get the content as a Variant. This can be > used for a whole range of different types of data including binary or maps > and lists. See the changes to qpid-send and qpid-receive for example of how > to use this. > > > Diffs > ----- > > /trunk/qpid/cpp/examples/messaging/drain.cpp 1510678 > /trunk/qpid/cpp/examples/messaging/map_receiver.cpp 1510678 > /trunk/qpid/cpp/examples/messaging/map_sender.cpp 1510678 > /trunk/qpid/cpp/examples/messaging/spout.cpp 1510678 > /trunk/qpid/cpp/include/qpid/messaging/Message.h 1510678 > /trunk/qpid/cpp/src/CMakeLists.txt 1510678 > /trunk/qpid/cpp/src/qpid/amqp/DataBuilder.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/amqp/DataBuilder.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/amqp/Encoder.h 1510678 > /trunk/qpid/cpp/src/qpid/amqp/Encoder.cpp 1510678 > /trunk/qpid/cpp/src/qpid/amqp/ListBuilder.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/amqp/ListBuilder.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/amqp/MapBuilder.h 1510678 > /trunk/qpid/cpp/src/qpid/amqp/MapBuilder.cpp 1510678 > /trunk/qpid/cpp/src/qpid/amqp/MessageEncoder.h 1510678 > /trunk/qpid/cpp/src/qpid/amqp/MessageEncoder.cpp 1510678 > /trunk/qpid/cpp/src/qpid/amqp/MessageReader.h 1510678 > /trunk/qpid/cpp/src/qpid/amqp/MessageReader.cpp 1510678 > /trunk/qpid/cpp/src/qpid/amqp/descriptors.h 1510678 > /trunk/qpid/cpp/src/qpid/amqp/typecodes.h 1510678 > /trunk/qpid/cpp/src/qpid/broker/amqp/Message.h 1510678 > /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp 1510678 > /trunk/qpid/cpp/src/qpid/broker/amqp/Translation.cpp 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.h 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/IncomingMessages.cpp 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/OutgoingMessage.cpp 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ReceiverImpl.h 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ReceiverImpl.cpp 1510678 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.cpp 1510678 > /trunk/qpid/cpp/src/qpid/messaging/Message.cpp 1510678 > /trunk/qpid/cpp/src/qpid/messaging/MessageImpl.h 1510678 > /trunk/qpid/cpp/src/qpid/messaging/MessageImpl.cpp 1510678 > /trunk/qpid/cpp/src/qpid/messaging/amqp/EncodedMessage.h 1510678 > /trunk/qpid/cpp/src/qpid/messaging/amqp/EncodedMessage.cpp 1510678 > /trunk/qpid/cpp/src/qpid/messaging/amqp/SenderContext.cpp 1510678 > /trunk/qpid/cpp/src/qpid/types/encodings.h PRE-CREATION > /trunk/qpid/cpp/src/tests/qpid-receive.cpp 1510678 > /trunk/qpid/cpp/src/tests/qpid-send.cpp 1510678 > > Diff: https://reviews.apache.org/r/13328/diff/ > > > Testing > ------- > > > Thanks, > > Gordon Sim > >
