Review: std.msgpack

[Masahiro Nakagawa]

On Sun, 20 Jun 2010 02:08:07 +0900, Robert Jacques <sandford at jhu.edu>  
wrote:

> On Sat, 19 Jun 2010 00:02:28 -0400, Masahiro Nakagawa  
> <repeatedly at gmail.com> wrote:
>
>> mp_Object is based on std.json.JSONValue.
>> Variant can't have mp_pack method.
>> Hmm..., I check Variant again.
>
> Well, std.json is an incomplete, alpha library that's buggy and  
> unreviewed. For example, the design is currently very C-ish with  
> structs, enums, functions and exceptions all being independent top level  
> constructs.

I checked Variant, but Variant[Variant] doesn't work... :(

>> If Phobos has std.serializer like Orange(or Doost.Serialization?),
>> I will implement MsgPackArchive.
>> But general serializer usually is low-performance and lacks some  
>> features.
>> So when user wants performance, user will use Packer and Unpacker  
>> directly.
>
> I'd disagree, with regard to performance: there's no reason why a  
> general (cycle free) serializer would be slower than your direct  
> routines. A serializer's only job is to pack/unpack complex objects  
> correctly: the key advantage is that polymorphic objects/struct only  
> have to support one custom serialization routine/mixin. Also, separating  
> physical-encoding  from logical-encoding allows for some very powerful  
> idioms. You could, for example, add cycle detection/support to msgpack  
> by having the serializer write in a micro-format on-top of msgpack.  
> Another, more important example is letting the user decide between an  
> array and a map representation for structs/objects. Using arrays for  
> objects is extremely brittle. If say, a new hire refactors a class from  
> A{ int x; int y;} to A{ int y; int x; } you now have an extremely hard  
> to find and track down logic bug in your program. To say nothing about  
> trying to upgrade an implementation. On the other hand, using maps for  
> objects adds a decent amount of overhead to the total serialized length.

Yeah, It's a implementation-dependent problem. So I wrote "usually".
If Phobos supports good serializer, I will remove Packer and  
direct-conversion Unpacker.

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

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

> (Returns: this to method chain. => Returns: self. i.e. for method  
> chaining )

Changed.

> Oh, as for packing real you'll need to use CustomFloat!(80) for cross  
> platform inter-op. Also, it shouldn't be @system: there's nothing  
> memory-unsafe about writing or reading them. Please don't abuse @system,  
> etc for meanings other their intended meanings.

I forgot this. CutstomFloat was already fixed by you and David in trunk.
I added 80bit floating point support for non-x86 environment.


Masahiro