> 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. > > Andrew Stitcher wrote: > 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.
No, they aren't in the external API, but then neither are these encoding constants. (They *could* be exposed - perhaps even should be - but that is a different change from the one I'm making which is simply to avoid having them redeclared in various different .cpp files). - Gordon ----------------------------------------------------------- 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 > >
