-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13328/#review24805
-----------------------------------------------------------


Nice work - I think this will make using structured data significantly easier.


/trunk/qpid/cpp/include/qpid/messaging/Message.h
<https://reviews.apache.org/r/13328/#comment48917>

    To be consistent with the other constructors there should be one that takes 
a Variant now.



/trunk/qpid/cpp/include/qpid/messaging/Message.h
<https://reviews.apache.org/r/13328/#comment48915>

    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().



/trunk/qpid/cpp/src/qpid/types/encodings.h
<https://reviews.apache.org/r/13328/#comment48923>

    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.


- Andrew Stitcher


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
> 
>

Reply via email to