1
\$\begingroup\$

I'm working on a small application that uses a WebService to get meta-data about television episodes. What I'd like to do is start building out a local cache of the series so that I can reduce the number of calls to the service. Data being stale is not a concern at all for this case. I've chosen to use an XML file to implement the local repository.

Here's the code:

public interface IVideo { int SeriesId { get; set; } string Name { get; set; } }

public class TVShow : IVideo { public int SeriesId { get; set; } public string Name { get; set; } }

public class IVideoCollection : List<IVideo>, IXmlSerializable
{
    public IVideoCollection() : base() { }

    #region IXmlSerializable
    public System.Xml.Schema.XmlSchema GetSchema() { return null; }

    public void ReadXml(XmlReader reader)
    {
        reader.ReadStartElement("IVideoCollection");
        while (reader.IsStartElement("IVideo"))
        {
            Type type = Type.GetType(reader.GetAttribute("AssemblyQualifiedName"));
            XmlSerializer serial = new XmlSerializer(type);

            reader.ReadStartElement("IVideo");
            this.Add((IVideo)serial.Deserialize(reader));
            reader.ReadEndElement();
        }
        reader.ReadEndElement();
    }

    public void WriteXml(XmlWriter writer)
    {
        foreach (IVideo vid in this)
        {
            writer.WriteStartElement("IVideo");
            writer.WriteAttributeString("AssemblyQualifiedName", vid.GetType().AssemblyQualifiedName);
            XmlSerializer xmlSerializer = new XmlSerializer(vid.GetType());
            xmlSerializer.Serialize(writer, vid);
            writer.WriteEndElement();
        }
    }
    #endregion
}

public class SeriesRepository
{
    public IVideoCollection Collection;
    public string RepoPath {get;set;}

    public SeriesRepository()
    {
        Collection = new IVideoCollection();
        RepoPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments), "IVideoCollection.xml");
        if (File.Exists(RepoPath))
        {
            var xmlSerializer = new XmlSerializer(Collection.GetType());
            var xmlReader = XmlReader.Create(new StreamReader(RepoPath));
            Collection = (IVideoCollection)xmlSerializer.Deserialize(xmlReader);
        }
    }

    public void SaveChanges()
    {
        var xmlSerializer = new XmlSerializer(Collection.GetType());
        StreamWriter writer = new StreamWriter(RepoPath);
        xmlSerializer.Serialize(writer, Collection);
        writer.Flush();
        writer.Close();
    }
}

I'm trying looking to see if there are any obvious points of failure that I haven't seen. Any feedback is appreciated.

The resulting XML looks like:

<?xml version="1.0" encoding="utf-8"?>
<IVideoCollection>
  <IVideo AssemblyQualifiedName="MyNamespace.TVShow, MyNameSpace, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null">
    <TVShow xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
      <SeriesId>55512345</SeriesId>
      <Name>This is the show name</Name>
    </TVShow>
  </IVideo>
</IVideoCollection>
\$\endgroup\$
1
\$\begingroup\$

I tend to prefer composition over inheritance, but I see why you inherited from List<T>, and I feel like it's okay, but you'll run into some trouble if you decide that some other type of collection would be better suited to your task. I would spend a little time thinking about what you'd need to do in your client code if you changed the implementation. It may be worth implementing IList by delegating to a private instance of List<IVideo>. Then again, it may not be.

XmlSerializer and StreamWriter both implement IDisposable, so it'd be better to put your usages of these into a using block. For example:

public void SaveChanges()
{
    using (var xmlSerializer = new XmlSerializer(Collection.GetType()))
    using (StreamWriter writer = new StreamWriter(RepoPath))
    {
        xmlSerializer.Serialize(writer, Collection);
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks @RubberDuck, I overlooked the fact that they both Implement IDisposable, and will update that right away. Your point about working with another type of collection is excellent, as the collection grows in size it will be more important for searching from a performance standpoint, I'll have to think on it a bit. \$\endgroup\$ – jaeckyl Nov 1 '15 at 14:40
  • \$\begingroup\$ You're welcome @jaeckyl. Good luck! \$\endgroup\$ – RubberDuck Nov 1 '15 at 14:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.