Refactor serialization of content to strategy

[ploeh]

This pull requests begins the refactoring work required to turn serialization and deserialization of XML Atom Content into an injectable Strategy.

The motivation for this is that it turns out that the current solution embedded in XmlAtomContent makes too many assumptions about the shape of events to be serialized. As one example, F# record types tend to follow other assembly and/or namespace patterns than assumed in the previous code base.

[sgryt]

Commits galore ;) I am not quite sure that I have a complete overview of this refactoring, even after studying the commits.

I understand the overall goal, and it makes sense to me, but it seems that a few asymmetries exist now (you probably have them listed for a coming refactor, but I'll mention them anyway):

• AtomFeed has no overload for writing AtomLink's with the serializer, but AtomLink has an overloaded WriteTo method that takes an IContentSerializer. I realise that the current implementation in AtomLink does not use the serializer, but it seems wrong to have different call patterns when an overload exist in AtomLink. Same comment for the ReadFrom call graph, and for the AtomLink's in AtomEntry.
• AtomEventStream has no constructor overload that accepts an IContentSerializer instance - which makes it hard to actually get to inject another strategy, where needed.

Other than that, it looks good to me, but I admit to not spending many hours on reviewing this PR.

[ploeh]

First of all: apologies for the huge pull request. Had I known it'd grow to that size, I'd have kept it smaller. I'll try not to repeat that :$

Re: asymmetric API

Your first issue is actually by design :p Really, I explicitly thought about that.

The reason that all the WriteTo methods have identical signatures is because they all implement the IXmlWritable interface. This means that there are implementations (such as AtomLink.WriteTo) that simply ignore the supplied serializer parameter, because they don't need it.

All writing of Atom XML is invariant, except for writing the actual content/payload. Thus, only XmlAtomContent actually needs the serializer, but that also causes all its callers to also need it, in order to be able to supply it to XmlAtomContent.

Having a common interface has valuable benefits, so I think that letting all classes implement that interface made lots of sense, even if some of them don't need the supplied serializer.

However, as is always the case with serialization and deserialization, there's an asymmetry, because a class can't implement a deserialization interface and still protect its invariants (that's the problem with the original description of the Memento pattern). For that reason, there's no ReadFrom (or Parse) interface, only concrete static methods.

Thus, since all the ReadFrom and Parse methods are static and concrete, I found it more appropriate to follow Postel's law and only require a client to supply the required arguments to each method.

That leads to a heterogeneous API when it comes to deserialization, but (hopefully) more usable methods overall.

Re: lack of injection

At this point, I thought the PR was already big enough, so I decided to stop here. Actual Constructor Injection is planned for a future PR.

HTH

[sgryt]

• Asymmetric API - which benefits have been reaped already, and which are expected to be gained going forward by having IXmlWritable implementations that are not used ?
• lack of injection: Roger that.

[ploeh]

The benefit of having a uniform IXmlWritable interface is that it has enabled the creation of two extension methods on that interface:

public static string ToXmlString(IXmlWritable, IContentSerializer)
public static string ToXmlString(IXmlWritable, IContentSerializer, XmlWriterSettings)

This reduces the number of unit tests required to maintain an easy way to turn any IXmlWritable into an XML string (which again is a feature a lot of the other unit tests (or should we say Facade Tests?) utilize).

The advantage of having an asymmetric deserialization API is, again, a reduced unit test foot print, because there's no reason to have concrete static methods taking redundant arguments - or, at least, I can't think of any good reason right now...