> On Aug. 7, 2013, 7:22 p.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/include/qpid/messaging/Message.h, line 167 > > <https://reviews.apache.org/r/13328/diff/2/?file=338325#file338325line167> > > > > I wonder if the API convention here is misused. The API is pretty > > consistent in using get...() for accessors and set...() for setters. > > > > plain content() is an accessor but doesn't match the convention. > > Perhaps to it should be getVariant() or maybe asVariant(). > > Gordon Sim wrote: > Yes, I only reluctantly went for content() because I couldn't think of > anything better (and getContent() is already used). It would need to to be > getContentAsVariant() to be meaningful and that seems a bit longwinded... or > maybe getContentObject()? > > Andrew Stitcher wrote: > I quite like getContentObject() - it indicates getContentString() which > we could add to disambiguate getContent() (it would just be another name for > it)
Ok, I'll change to getContentObject(). I'll add getContentBytes() as an alias for getContent() to make it clear that its not necessarily a textual 'string', but just a sequence of chars. > 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. 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. - 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 > >