Should I create a separate class for this?

Question

My project contains a lot of classes, a bunch of which can be described by XML files. Don't worry, it's not the logic or implementation in XML. It's a game, and an example would be that a game tile can be defined in XML, the image file, animation frames, etc.

I'm going to end up having a bunch of functions that look like this:

public static Foo FromXml(ref XmlTextReader reader) { ... }

The question is this: should these functions be contained in their own matching class, for example the one above would be Foo.FromXml. Or, should I make a separate class for reading things from files? There seem to be two general guidelines competing here:

1. A class should to know everything about one thing - itself.
2. A class should only have one reason to change.

First of all, I don't really understand the second one, because "reason" is quite vague. The first guideline suggests put each reader in the related class. The second says to make one class dedicated to reading xml files. But the pros and cons are debatable. On the one hand, each class could contain its own reader, so there aren't a dozen classes referenced. On the other hand, every class would have to include System.Xml, and if I change my xml format, things could change in multiple files (but I don't think that's too bad).

I know the most important rule is "use your brain" and there is no such thing as a "correct" solution, only a good and working one. So what do you think would be more readable, or better yet, maintainable?

edit: to clarify, the classes can be totally unrelated. Since it's a game, one may be the sprite animation class, one may define enemy behavior, one might define the map layout or properties. So inheritance has nothing to do with this.

Answer 1

Are the FromXml(...) functions identical? Assuming they are I would be putting it in a common library area, because it will make maintaining them a lot easier as there will be no code duplication. The code should still be neat too

SomeObject o = (SomeObject)Foo.FromXml(reader);

EDIT: Or, possibly, make some base abstract class which just has the FromXml / ToXml functions, then have all classes that want to use those functions inherit from the abstract class.

Answer 2

having a static base class method for all inheriting classes & maintaining proper polymorphism is difficult. You could however remove the static-ness of the method, and have an InitializeFromXml method which would essentially allow you to populate your class from the xml. Though I typically don't care for public Initialize methods, this tends to be better for polymorphism.

here's an example. it's a bit much for a small object like this (and I rarely ever actually use xml serialization, but would load it into an xml document and pull out what I need for deserialization assignments), but when things scale, are inherited, and generally more complex, it allows you to reuse quite a bit more:

public class CustomObject {
    public string AValue { get; set; }
    public bool BValue { get; set; }
    protected IXmlConfiguration Config = new CustomObjectConfig( );

    public virtual string ToXml( ) {
        return Config.ToXml( this );
    }

    public virtual void InitializeFromXml( string xml ) {
        Config.FromXml( xml );
        AValue = ((CustomObjectConfig)Config).A;
        BValue = ((CustomObjectConfig)Config).B;
    }
}

public interface IXmlConfiguration {
    void FromXml( string xml );
    string ToXml( object instance );
}

[XmlRoot( "CustomObject" )]
public class CustomObjectConfig : IXmlConfiguration {
    [XmlElement( "AValue" )]
    public string A { get; set; }
    [XmlAttribute( "bvalue" )]
    public bool B { get; set; }

    public void FromXml( string xml ) {
        byte[] bytes = Encoding.UTF8.GetBytes( xml );
        using ( MemoryStream ms = new MemoryStream( bytes ) ) {
            XmlSerializer xs = new XmlSerializer( typeof( CustomObjectConfig ) );
            CustomObjectConfig cfg = (CustomObjectConfig)xs.Deserialize( ms );
            A = cfg.A;
            B = cfg.B;
        }            
    }

    public string ToXml( object instance ) {
        string xml = null;
        if ( instance is CustomObject ) {
            CustomObject val = (CustomObject)instance;
            A = val.AValue;
            B = val.BValue;
            using ( MemoryStream ms = new MemoryStream( ) ) {
                XmlSerializer xs = new XmlSerializer( typeof( CustomObjectConfig ) );
                xs.Serialize( ms, this );
                ms.Seek( 0, 0 );
                byte[] bytes = ms.ToArray( );
                xml = Encoding.UTF8.GetString( bytes );
            }
        }
        return xml;
    }
}

The reason I'd favor this approach rather than creating xml serializable objects are because

1. xml serialization typically requires you structure your class one way, and you will typically then use that class another way.
2. it's a bit easier than having huge stacks of xml include attributes (may be called something different) everywhere which would allow the polymorphic serialization of derived types.