Request for pre-review: std.serialization/orange

[Jacob Carlborg]

On 2011-10-01 06:29, Robert Jacques wrote:
> On Thu, 29 Sep 2011 14:58:30 -0400, Jacob Carlborg <doob at me.com> wrote:
>
>> I would like to have some form of pre-review of my serialization library
>> Orange for later inclusion in Phobos as std.serialization (or similar).
>
> [snip]
>
> (1)
> The first example in class Serializer is:
>
> auto serializer = new Serializer;
>
> Shouldn't it be
>
> auto serializer = new Serializer(archive);

You're right, thanks.

> (2)
> orange.serialization.archives.XmlArchive need to be documented.

I was hoping the Archive interface and the Base abstract class would be 
enough.

> (3)
> if Archive.Array (which is poorly named btw) "is a type independent
> representation of an array" then why does it contain an elementSize field?

Suggestions for other names are welcome. Perhaps it was poorly worded, 
but what I mean is that this type can represent all array types.

> (3a)
> Also by the time archiving is called, isSliceOf should always return
> false.

Why is that?

> That this function exists speaks to design problems both large
> and small. On the small scale, isSliceOf indicates that you are testing
> every array against every other array for slicing, which I hope is not
> the case.

I do. How would I otherwise discover if an array is a slice of another 
array or not?

> On the large scale, all the alias/object/pointer/arrays/etc
> resolutions should be done by the serializer not the archive. The
> archive should only be responsible for converting the internal
> representation of the serializer to JSON/XML/YAML/etc.

isSliceOf is never called by any existing archive. Perhaps Slice and 
Array should be moved to the serializer module. I can also remove 
isSliceOf from Base. I think I was trying to keep the archives 
independent of the serializer, in the sense that the archives  never 
should have to import the serializer. That's not currently that case so 
these structs could be moved.

> (3b)
> Given that Slice has an ID field, why doesn't array.

That's a good question. I don't think it's used at all. archiveSlice 
takes a Slice and a sliceId. This field should be removed.

> (4)
> Why have an Archive Interface and a Base class with common
> functionality? Why not an abstract class? Also, 'Base' isn't an
> acceptable class name for a Phobos module. Use 'ArchiveBase' or
> something more unique instead.

As you can see Base is a template class and therefore is dependent on 
the archive type. My goal was that Serializer should not be a template 
class.

I thought it was unnecessary to repeat the module name in the class 
name. But in this case it might be a good idea. Or perhaps call it 
AbstractArchive.

-- 
/Jacob Carlborg