6
\$\begingroup\$

I'm looking for remarks on pretty much anything. This has been my first project that's meant for use by others and I'd like to have it as clean as possible. I will post the main source code here but because of the size you might prefer reading it on Github instead.

The code should be adequately documented so I don't think a lot of explanation is needed. The project itself is a wrapper for an API to translate XML requests and responses into simple C# objects.

Any advice is welcome!

TVDB

namespace TVDBSharp {
    /// <summary>
    /// The main class which will handle all user interaction.
    /// </summary>
    public class TVDB {
        private readonly IDataProvider _dataProvider;

        /// <summary>
        /// Creates a new instance with the provided API key and dataProvider.
        /// </summary>
        /// <param name="apiKey">The API key provided by TVDB.</param>
        /// <param name="dataProvider">Specify your own <see cref="IDataProvider"/> instance.</param>
        public TVDB(string apiKey, IDataProvider dataProvider) {
            _dataProvider = dataProvider;
            _dataProvider.ApiKey = apiKey;
        }

        /// <summary>
        /// Creates a new instance with the provided API key and standard <see cref="IDataProvider"/>.
        /// </summary>
        /// <param name="apiKey">The API key provided by TVDB.</param>
        public TVDB(string apiKey) {
            _dataProvider = new DataProvider { ApiKey = apiKey };
        }

        /// <summary>
        /// Search for a show in the database.
        /// </summary>
        /// <param name="query">Query that identifies the show.</param>
        /// <param name="results">Maximal amount of results in the returning set. Default is 5.</param>
        /// <returns>Returns a list of shows.</returns>
        public List<Show> Search(string query, int results = 5) {
            return new Builder(_dataProvider).Search(query, results);
        }

        /// <summary>
        /// Get a specific show based on its ID.
        /// </summary>
        /// <param name="showId">ID of the show.</param>
        /// <returns>Returns the corresponding show.</returns>
        public Show GetShow(string showId) {
            return new Builder(_dataProvider).BuildShow(showId);
        }
    }
}

Models.DAO.IDataProvider

namespace TVDBSharp.Models.DAO {
    /// <summary>
    /// Defines a Dataprovider API.
    /// </summary>
    public interface IDataProvider {
        /// <summary>
        /// The API key provided by TVDB.
        /// </summary>
        string ApiKey { get; set; }

        /// <summary>
        /// Retrieves the show with the given id and returns the corresponding XML tree.
        /// </summary>
        /// <param name="showID">ID of the show you wish to lookup.</param>
        /// <returns>Returns an XML tree of the show object.</returns>
        XDocument GetShow(string showID);

        /// <summary>
        /// Returns an XML tree representing a search query for the given parameter.
        /// </summary>
        /// <param name="query">Query to perform the search with.</param>
        /// <returns>Returns an XML tree of a search result.</returns>
        XDocument Search(string query);
    }
}

Models.DAO.DataProvider

namespace TVDBSharp.Models.DAO {
    /// <summary>
    /// Standard implementation of the <see cref="IDataProvider"/> interface.
    /// </summary>
    public class DataProvider : IDataProvider {
        public string ApiKey { get; set; }

        public XDocument GetShow(string showID) {
            var web = new WebClient();
            var response = web.DownloadString(new StringBuilder("http://thetvdb.com/api/").Append(ApiKey).Append("/series/").Append(showID).Append("/all/").ToString());
            return XDocument.Parse(response);
        }

        public XDocument Search(string query) {
            var web = new WebClient();
            var response = web.DownloadString(new StringBuilder("http://thetvdb.com/api/GetSeries.php?seriesname=").Append(query).ToString());
            return XDocument.Parse(response);
        }
    }
}

Models.Builder

namespace TVDBSharp.Models {
    /// <summary>
    /// Provides builder classes for complex entities.
    /// </summary>
    public class Builder {
        private readonly IDataProvider _dataProvider;

        /// <summary>
        /// Initializes a new Builder object with the given <see cref="IDataProvider"/>.
        /// </summary>
        /// <param name="dataProvider">The DataProvider used to retrieve XML responses.</param>
        public Builder(IDataProvider dataProvider) {
            _dataProvider = dataProvider;
        }

        /// <summary>
        /// Builds a show object from the given show ID.
        /// </summary>
        /// <param name="showID">ID of the show to serialize into a <see cref="Show"/> object.</param>
        /// <returns>Returns the Show object.</returns>
        public Show BuildShow(string showID) {
            var builder = new ShowBuilder(_dataProvider.GetShow(showID));
            return builder.GetResult();
        }

        /// <summary>
        /// Returns a list of <see cref="Show"/> objects that match the given query.
        /// </summary>
        /// <param name="query">Query the search is performed with.</param>
        /// <param name="results">Maximal amount of shows the resultset should return.</param>
        /// <returns>Returns a list of show objects.</returns>
        public List<Show> Search(string query, int results) {
            var shows = new List<Show>(results);
            var doc = _dataProvider.Search(query);

            foreach (var element in doc.Descendants("Series").Take(results)) {
                var id = element.GetXmlData("seriesid");
                var response = _dataProvider.GetShow(id);
                shows.Add(new ShowBuilder(response).GetResult());
            }

            return shows;
        }

        private class ShowBuilder {
            private Show _show;

            public ShowBuilder(XDocument doc) {
                _show = new Show();
                _show.ID = doc.GetSeriesData("id");
                _show.ImdbID = doc.GetSeriesData("IMDB_ID");
                _show.Name = doc.GetSeriesData("SeriesName");
                _show.Language = doc.GetSeriesData("Language");
                _show.Network = doc.GetSeriesData("Network");
                _show.Description = doc.GetSeriesData("Overview");
                _show.Rating = string.IsNullOrWhiteSpace(doc.GetSeriesData("Rating"))
                                   ? (double?) null
                                   : Convert.ToDouble(doc.GetSeriesData("Rating"),
                                                      System.Globalization.CultureInfo.InvariantCulture);
                _show.RatingCount = string.IsNullOrWhiteSpace(doc.GetSeriesData("RatingCount"))
                                        ? (int?) null
                                        : Convert.ToInt32(doc.GetSeriesData("RatingCount"));
                _show.Runtime = string.IsNullOrWhiteSpace(doc.GetSeriesData("Runtime"))
                                    ? (int?) null
                                    : Convert.ToInt32(doc.GetSeriesData("Runtime"));
                _show.Banner = doc.GetSeriesData("banner");
                _show.Fanart = doc.GetSeriesData("fanart");
                _show.LastUpdated = string.IsNullOrWhiteSpace(doc.GetSeriesData("lastupdated"))
                                        ? 0L
                                        : Convert.ToInt64(doc.GetSeriesData("lastupdated"));
                _show.Poster = doc.GetSeriesData("poster");
                _show.Zap2ItID = doc.GetSeriesData("zap2it_id");
                _show.FirstAired = Utils.ParseDate(doc.GetSeriesData("FirstAired"));
                _show.AirTime = string.IsNullOrWhiteSpace(doc.GetSeriesData("Airs_Time"))
                                    ? (TimeSpan?) null
                                    : Utils.ParseTime(doc.GetSeriesData("Airs_Time"));
                _show.AirDay = string.IsNullOrWhiteSpace(doc.GetSeriesData("Airs_DayOfWeek"))
                                   ? (DayOfWeek?) null
                                   : (DayOfWeek) Enum.Parse(typeof(DayOfWeek), doc.GetSeriesData("Airs_DayOfWeek"));
                _show.Status = string.IsNullOrWhiteSpace(doc.GetSeriesData("Status"))
                                   ? (Status?) null
                                   : (Status) Enum.Parse(typeof(Status), doc.GetSeriesData("Status"));
                _show.ContentRating = Utils.GetContentRating(doc.GetSeriesData("ContentRating"));
                _show.Genres = new List<string>(doc.GetSeriesData("Genre").Split('|'));
                _show.Actors = new List<string>(doc.GetSeriesData("Actors").Split('|'));
                _show.Episodes = new EpisodeBuilder(doc).BuildEpisodes();
            }

            public Show GetResult() {
                return _show;
            }
        }

        private class EpisodeBuilder {
            private XDocument _doc;

            public EpisodeBuilder(XDocument doc) {
                _doc = doc;
            }

            public List<Episode> BuildEpisodes() {
                var result = new List<Episode>();

                foreach (var episode in _doc.Descendants("Episode")) {
                    var ep = new Episode {
                        ID = episode.GetXmlData("id"),
                        Title = episode.GetXmlData("EpisodeName"),
                        Description = episode.GetXmlData("Overview"),
                        EpisodeNumber =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("EpisodeNumber"))
                                ? (int?) null
                                : Convert.ToInt32(episode.GetXmlData("EpisodeNumber")),
                        Director = episode.GetXmlData("Director"),
                        FileName = episode.GetXmlData("filename"),
                        FirstAired =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("FirstAired"))
                                ? (DateTime?) null
                                : Utils.ParseDate(episode.GetXmlData("FirstAired")),
                        GuestStars = new List<string>(episode.GetXmlData("GuestStars").Split('|')),
                        ImdbID = episode.GetXmlData("IMDB_ID"),
                        Language = episode.GetXmlData("Language"),
                        LastUpdated =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("lastupdated"))
                                ? 0L
                                : Convert.ToInt64(episode.GetXmlData("lastupdated")),
                        Rating =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("Rating"))
                                ? (double?) null
                                : Convert.ToDouble(episode.GetXmlData("Rating"),
                                                   System.Globalization.CultureInfo.InvariantCulture),
                        RatingCount =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("RatingCount"))
                                ? (int?) null
                                : Convert.ToInt32(episode.GetXmlData("RatingCount")),
                        SeasonID = episode.GetXmlData("seasonid"),
                        SeasonNumber =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("SeasonNumber"))
                                ? (int?) null
                                : Convert.ToInt32(episode.GetXmlData("SeasonNumber")),
                        SeriesID = episode.GetXmlData("seriesid"),
                        ThumbHeight =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("thumb_height"))
                                ? (int?) null
                                : Convert.ToInt32(episode.GetXmlData("thumb_height")),
                        ThumbWidth =
                            string.IsNullOrWhiteSpace(episode.GetXmlData("thumb_width"))
                                ? (int?) null
                                : Convert.ToInt32(episode.GetXmlData("thumb_width")),
                        TmsExport = episode.GetXmlData("tms_export"),
                        Writers = new List<string>(episode.GetXmlData("Writer").Split('|'))
                    };

                    result.Add(ep);
                }

                return result;
            }
        }
    }
}

Utilities.Extensions

namespace TVDBSharp.Utilities {
    /// <summary>
    /// Extension methods used to simplify data extraction.
    /// </summary>
    public static class Extensions {
        /// <summary>
        /// Retrieves a value from an XML tree representing a show.
        /// </summary>
        /// <param name="doc">XML tree representing a show.</param>
        /// <param name="element">Name of the element with the data.</param>
        /// <returns>Returns the value corresponding to the given element name.</returns>
        /// <exception cref="XmlSchemaException">Thrown when the element doesn't exist or the XML tree is incorrect.</exception>
        public static string GetSeriesData(this XDocument doc, string element) {
            var root = doc.Element("Data");
            if (root != null) {
                var xElement = root.Element("Series");
                if (xElement != null) {
                    var result = xElement.Element(element);
                    if (result != null) {
                        return result.Value;
                    }
                    throw new XmlSchemaException("Could not find element <" + element + ">");
                }
                throw new XmlSchemaException("Could not find element <Series>");
            }
            throw new XmlSchemaException("Could not find element <Data>");
        }

        /// <summary>
        /// Retrieves a value from an XML tree.
        /// </summary>
        /// <param name="xmlObject">The given XML (sub)tree.</param>
        /// <param name="element">Name of the element with the data.</param>
        /// <returns>Returns the value corresponding to the given element name;</returns>
        /// <exception cref="XmlSchemaException">Thrown when the element doesn't exist.</exception>
        public static string GetXmlData(this XElement xmlObject, string element) {
            var result = xmlObject.Element(element);
            if (result != null) {
                return result.Value;
            }

            throw new XmlSchemaException("Element <" + element + "> could not be found.");
        }
    }
}

Utilities.Utils

namespace TVDBSharp.Utilities {
    /// <summary>
    /// Provides static utility methods.
    /// </summary>
    public static class Utils {
        /// <summary>
        /// Parses a string of format yyyy-mm-dd to a <see cref="DateTime"/> object.
        /// </summary>
        /// <param name="value">String to be parsed.</param>
        /// <returns>Returns a <see cref="DateTime"/> representation.</returns>
        public static DateTime ParseDate(string value) {
            var date = value.Split('-');
            return new DateTime(Convert.ToInt32(date[0]), Convert.ToInt32(date[1]), Convert.ToInt32(date[2]));
        }

        /// <summary>
        /// Parses a string of format hh:mm tt to a <see cref="TimeSpan"/> object.
        /// </summary>
        /// <param name="value">String to be parsed.</param>
        /// <returns>Returns a <see cref="TimeSpan"/> representation.</returns>
        public static TimeSpan ParseTime(string value) {
            var hour = Convert.ToInt32(value.Substring(0, value.IndexOf(':')));
            var minute = Convert.ToInt32(value.Substring(value.IndexOf(':') + 1, 2));
            var daypart = value.Substring(value.Length - 2);
            if (daypart == "PM") {
                hour += 12;
            }

            return new TimeSpan(hour, minute, 0);
        }

        /// <summary>
        /// Translates the incoming string to a <see cref="ContentRating"/> enum, if applicable.
        /// </summary>
        /// <param name="rating">The rating in string format.</param>
        /// <returns>Returns the appropriate <see cref="ContentRating"/> value.</returns>
        /// <exception cref="ArgumentException">Throws an exception if no conversion could be applied.</exception>
        public static ContentRating GetContentRating(string rating) {
            switch (rating) {
                case "TV-14":
                    return ContentRating.TV14;

                case "TV-PG":
                    return ContentRating.TVPG;

                case "TV-Y":
                    return ContentRating.TVY;

                case "TV-Y7":
                    return ContentRating.TVY7;

                case "TV-G":
                    return ContentRating.TVG;

                case "TV-MA":
                    return ContentRating.TVMA;

                default:
                    return ContentRating.Unknown;
            }
        }
    }
}

If you've made it this far, here is the entire test setup to review as well!

Models.Conversion

namespace Tests.Models {
    /// <summary>
    /// A helper class to translate an object to its XML value and vice versa.
    /// </summary>
    public class Conversion {
        /// <summary>
        /// The XML representation for either an element tag or a value.
        /// </summary>
        public string XmlValue { get; set; }

        /// <summary>
        /// The object representation for either a property or a value.
        /// </summary>
        public string ObjValue { get; set; }

        /// <summary>
        /// Constructs a new object with the given values.
        /// </summary>
        /// <param name="xmlValue">XML Value (see <see cref="XmlValue"/>).</param>
        /// <param name="objValue">Object Value (see <see cref="ObjValue"/>).</param>
        public Conversion(string xmlValue, string objValue) {
            XmlValue = xmlValue;
            ObjValue = objValue;
        }

        public override bool Equals(object obj) {
            if (ReferenceEquals(null, obj))
                return false;
            if (ReferenceEquals(this, obj))
                return true;
            if (obj.GetType() != this.GetType())
                return false;
            return Equals((Conversion) obj);
        }

        protected bool Equals(Conversion other) {
            return string.Equals(XmlValue, other.XmlValue) && string.Equals(ObjValue, other.ObjValue);
        }

        public override int GetHashCode() {
            unchecked {
                return ((XmlValue != null ? XmlValue.GetHashCode() : 0) * 397) ^ (ObjValue != null ? ObjValue.GetHashCode() : 0);
            }
        }
    }
}

Models.Data

namespace Tests.Models {
    /// <summary>
    /// Simulation of the real XML tree.
    /// </summary>
    [XmlRoot("Data")]
    public class Data {
        /// <summary>
        /// The XML tree's show object.
        /// </summary>
        [XmlElement("Series")]
        public TestShow TestShow { get; set; }
    }
}

TestDataProvider

namespace Tests {
    /// <summary>
    /// Dataprovider used for testing. This class generates XML trees to be used for parsing tests.
    /// </summary>
    public class TestDataProvider : IDataProvider {
        private readonly TestData _data;

        public string ApiKey { get; set; }

        /// <summary>
        /// Initializes a new instance with the provided testing data.
        /// </summary>
        /// <param name="data">Mocking data of type <see cref="TestData"/>.</param>
        public TestDataProvider(TestData data) {
            _data = data;
        }

        public XDocument GetShow(string showID) {
            var showData = _data.GetShowData();
            var episodeData = _data.GetEpisodeData();
            var show = new Data { TestShow = new TestShow() };

            // Dynamically create the show object
            foreach (var key in showData.Keys) {
                var prop = show.TestShow.GetType().GetProperty(key.XmlValue);
                prop.SetValue(show.TestShow, showData[key].XmlValue, null);
            }

            // Add episodes to the show object
            show.TestShow.Episodes = new List<TestEpisode>();
            foreach (var ep in episodeData) {
                var newEpisode = new TestEpisode();

                foreach (var key in ep.Keys) {
                    var prop = newEpisode.GetType().GetProperty(key.XmlValue);
                    prop.SetValue(newEpisode, ep[key].XmlValue, null);
                }

                show.TestShow.Episodes.Add(newEpisode);
            }

            // Pull the created object trough an XML serializer
            var serializer = new XmlSerializer(show.GetType());
            string xml;

            using (var writer = new StringWriter()) {
                serializer.Serialize(writer, show);
                xml = writer.ToString();
            }

            return XDocument.Parse(xml);
        }

        public XDocument Search(string query) {
            throw new NotImplementedException();
        }
    }
}

MainTests

namespace Tests {
    /// <summary>
    /// A collection of the most important tests which test the complete workflow excluding connecting to the web service.
    /// </summary>
    [TestClass]
    public class MainTests {
        private TestData _data;
        private IDataProvider _dataProvider;

        /// <summary>
        /// Initializes the test with mock data. See <see cref="TestData"/> for more information.
        /// </summary>
        [TestInitialize]
        public void Initialize() {
            _data = new TestData();
            _dataProvider = new TestDataProvider(_data);
        }

        /// <summary>
        /// Test the retrieval of a show. A <see cref="TestShow"/> object is created
        /// to accurately represent the XML tree of a show.
        ///
        /// Afterwards the <see cref="TVDBSharp.Models.Builder"/> is called
        /// to parse this into a <see cref="TVDBSharp.Models.Show"/> object.
        ///
        /// This process includes creating <see cref="TVDBSharp.Models.Episode"/> objects.
        /// Finally every property is being tested to have the expected outcome a
        /// as detailed in <see cref="TestData"/>.
        /// </summary>
        [TestMethod]
        public void GetShow() {
            // Pull XML tree trough the show builder
            var builder = new Builder(_dataProvider);
            var result = builder.BuildShow(_data.GetShowData().Keys.FirstOrDefault(x => x.XmlValue == "id").XmlValue);

            var showData = _data.GetShowData();
            var episodeData = _data.GetEpisodeData();

            // Assert equality between value conversions for show data
            foreach (var key in showData.Keys) {
                var prop = result.GetType().GetProperty(key.ObjValue);
                Assert.IsTrue(prop.GetValue(result).ToString() == showData[key].ObjValue, "!Show object! Property: " + prop.Name + " ;Actual object value: " + prop.GetValue(result) + " ;Expected value: " + showData[key].ObjValue);
            }

            // Assert equality between value conversion for episode data
            for (var i = 0; i < result.Episodes.Count; i++) {
                var currentEpisode = result.Episodes[i];
                var dic = episodeData[i];

                foreach (var key in dic.Keys) {
                    var prop = currentEpisode.GetType().GetProperty(key.ObjValue);

                    // Checks whether or not we're dealing with a list
                    // ToString() method on lists will not show the values and are therefore not suited for comparison
                    // That's why we manually check the entries
                    if (new List<string> { "Actors", "Genres", "GuestStars", "Writers" }.Contains(key.ObjValue)) {
                        foreach (var entry in dic[key].XmlValue.Split('|')) {
                            Assert.IsTrue(((List<string>) prop.GetValue(currentEpisode)).Contains(entry), "!List object! Property: " + prop.Name + " ;Actual object value: " + string.Join(", ", (List<string>) prop.GetValue(currentEpisode)) + ";Expected value: " + dic[key].XmlValue);
                        }
                    }

                    var value = prop.GetValue(currentEpisode).ToString();
                    var expected = dic[key].ObjValue;
                    Assert.IsTrue(value == expected, "!Episode object! Property: " + prop.Name + " ;Actual object value: " + prop.GetValue(currentEpisode) + " ;Expected value: " + dic[key].ObjValue);
                }
            }
        }
    }
}

A few helper classes that are just property classes or encapsulate testing data are left out, you can view those on the github page.

\$\endgroup\$
  • 3
    \$\begingroup\$ IMO there's way too much comments, it really slows down the reading. Things like builders don't need comments or even your unit tests. They should be straightforward enough so the code can speak for itself. \$\endgroup\$ – Pierre-Luc Pineault Sep 18 '13 at 18:55
  • 1
    \$\begingroup\$ @Pierre-LucPineault: the XML comments are there for intellisense usage by external developers. Surely that's a good reason, no? The unit test was pretty complex which is why I added comments to clarify it. \$\endgroup\$ – Jeroen Vannevel Sep 18 '13 at 19:27
6
\$\begingroup\$

Some minor stuff for now:

  1. Any specific reason to use StringBuilder rather than string.Format()? I personally find

    string.Format("http://thetvdb.com/api/{0}/series/{1}/all/", ApiKey, showID)
    

    easier to read than

    new StringBuilder("http://thetvdb.com/api/").Append(ApiKey).Append("/series/").Append(showID).Append("/all/").ToString()
    

    The structure of the resulting string is much easier to see with string.Format (and it's also less code).

  2. You do something like this fairly often:

    string.IsNullOrWhiteSpace(doc.GetSeriesData("Runtime"))
            ? (int?) null
            : Convert.ToInt32(doc.GetSeriesData("Runtime"));
    

    You could look at encapsulating this in an extension method like this

    T GetSeriesData(this XDocument doc, string key, Func<string, T> converter);
    
\$\endgroup\$
  • \$\begingroup\$ Again a very spot on answer! I'll accept it as well because I don't expect there to be any traction for this question anymore. Thank you for the time to look over these two programs! \$\endgroup\$ – Jeroen Vannevel Nov 24 '13 at 13:27

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.