Review: std.msgpack

[Masahiro Nakagawa]

Sorry for the delay.


On Tue, 22 Jun 2010 13:47:19 +0900, Robert Jacques <sandford at jhu.edu>  
wrote:

>>>>> Other little things,
>>>>> -Why isn't nil mapped to null?
>>>>
>>>> Where?
>>>
>>> MsgPack has a nil type which I assume is equivalent to null. So why  
>>> isn't it simply included in the object serializer (and arguably also  
>>> in the array and map serializer as well)?
>>
>> Sorry, I can't understand this statement. Nullable types(class, array  
>> and associative-array)
>> is serialized to nil if null.
>
> Sorry. The comment was about redundancy. i.e. packNil <=> pack(null).

I added pointer type (de)serialization.

>>> For that matter, packNil, packTrue, packFalse, etc are all redundant  
>>> with pack(T).
>>
>> I removed packTrue, packFalse.
>>
>>> Furthermore, stream, packArray, packMap and packRaw all should be  
>>> internal routines. (Though, a rawPack(T)(T value) routine might not go  
>>> amiss)
>>
>> I moved packRaw to internal method, but packArray and packMap should be  
>> public.
>> MessagePack's array and map can contain any type.
>
> I can see the usefulness of the functionality, but for an end-user API,  
> it is brittle and error-prone. For example:  
> p.packArray(3).pack(5).pack("six"); produces an incorrect msgPack  
> stream. A better syntax would be: p.pack(5,"six"); or  
> p.packArray(5,"six"); I know you're using pack(Types...) to mean  
> pack(Types[0]).pack(Types[1])... but using it for variable type arrays  
> is worth considering. For packMap, pack( ["key": "value"], [5:"six"] )  
> could work, but packMap( ["key": "value"], [5:"six"] ) or  
> packMap("key","value",5,"six") would avoid confusion with pack(Types...).

I have thought about this point. In the result, I changed (Un)Packer API.
It's true that your suggested syntax is better.
Old packArray was renamed to beginArray and new packArray serializes
container type using variadic arguments. Of course, Map too.

-----
/* old */
packer.packArray(2).pack(true, 10);

/* new */
packer.packArray(true, 10);
// == packer.beginArray(2).pack(true, 10)
-----

Thanks for pointing out this.


Masahiro