Need some suggestions on my softwares architecture. [Code review]

Question

I'm making an open source C# library for other developers to use. My key concern is ease of use. This means using intuitive names, intuitive method usage and such.

This is the first time I've done something with other people in mind, so I'm really concerned about the quality of the architecture. Plus, I wouldn't mind learning a thing or two. :)

I have three classes: Downloader, Parser and Movie

I was thinking that it would be best to only expose the Movie class of my library and have Downloader and Parser remain hidden from invocation.

Ultimately, I see my library being used like this.

using FreeIMDB;

public void Test()
{
    var MyMovie = Movie.FindMovie("The Matrix");
    //Now MyMovie would have all it's fields set and ready for the big show.
}

Can you review how I'm planning this, and point out any wrong judgement calls I've made and where I could improve.

Remember, my main concern is ease of use.

Movie.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Drawing;


namespace FreeIMDB
{
    public class Movie
    {
        public Image Poster { get; set; }
        public string Title { get; set; }
        public DateTime ReleaseDate { get; set; }
        public string Rating { get; set; }
        public string Director { get; set; }
        public List<string> Writers { get; set; }
        public List<string> Genres { get; set; }
        public string Tagline { get; set; }
        public string Plot { get; set; }
        public List<string> Cast { get; set; }
        public string Runtime { get; set; }
        public string Country { get; set; }
        public string Language { get; set; }

        public Movie FindMovie(string Title)
        {
            Movie film = new Movie();
            Parser parser = Parser.FromMovieTitle(Title);

            film.Poster = parser.Poster();
            film.Title = parser.Title();
            film.ReleaseDate = parser.ReleaseDate();
            //And so an so forth.
        }

        public Movie FindKnownMovie(string ID)
        {
            Movie film = new Movie();
            Parser parser = Parser.FromMovieID(ID);

            film.Poster = parser.Poster();
            film.Title = parser.Title();
            film.ReleaseDate = parser.ReleaseDate();
            //And so an so forth.
        }
    }
}

Parser.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using HtmlAgilityPack;

namespace FreeIMDB
{
    /// <summary>
    /// Provides a simple, and intuitive way for searching for movies and actors on IMDB.
    /// </summary>
    class Parser
    {
        private Downloader downloader = new Downloader();                
        private HtmlDocument Page;

        #region "Page Loader Events"
        private Parser()
        {

        }

        public static Parser FromMovieTitle(string MovieTitle)
        {
            var newParser = new Parser();
            newParser.Page = newParser.downloader.FindMovie(MovieTitle);
            return newParser;
        }

        public static Parser FromActorName(string ActorName)
        {
            var newParser = new Parser();
            newParser.Page = newParser.downloader.FindActor(ActorName);
            return newParser;
        }

        public static Parser FromMovieID(string MovieID)
        {
            var newParser = new Parser();
            newParser.Page = newParser.downloader.FindKnownMovie(MovieID);
            return newParser;
        }

        public static Parser FromActorID(string ActorID)
        {
            var newParser = new Parser();
            newParser.Page = newParser.downloader.FindKnownActor(ActorID);
            return newParser;
        }
        #endregion

        #region "Page Parsing Methods"
        public string Poster()
        {
            //Logic to scrape the Poster URL from the Page element of this.
            return null;
        }

        public string Title()
        {
            return null;
        }

        public DateTime ReleaseDate()
        {
            return null;
        }        
        #endregion        
    }
}

-----------------------------------------------

Do you guys think I'm heading towards a good path, or am I setting myself up for a world of hurt later on?

My original thought was to separate the downloading, the parsing and the actual populating to easily have an extensible library. Imagine if one day the website changed its HTML, I would then only have to modifiy the parsing class without touching the Downloader.cs or Movie.cs class.

Thanks for reading and for helping!

Any other ideas?

Answer 1

Your API is mostly static, meaning you are setting yourself up for maintainability issues in the future. This is because the static methods are actually singletons, which have some significant drawbacks.

I suggest striving for a more instance-based, decoupled approach. This will naturally separate the definition of each operation from its implementation, leaving room for extensibility and configuration. An API's ease-of-use is measured not only by its public surface, but also by its adaptability.

Here is how I would go about designing this system. First, define something which is responsible for fetching movies:

public interface IMovieRepository
{
    Movie FindMovieById(string id);

    Movie FindMovieByTitle(string title);
}

Next, define something which is responsible for downloading HTML documents:

public interface IHtmlDownloader
{
    HtmlDocument DownloadHtml(Uri uri);
}

Then, define a repository implementation which uses a downloader:

public class MovieRepository : IMovieRepository
{
    private readonly IHtmlDownloader _downloader;

    public MovieRepository(IHtmlDownloader downloader)
    {
        _downloader = downloader;
    }

    public Movie FindMovieById(string id)
    {
        var idUri = ...build URI...;

        var html = _downloader.DownloadHtml(idUri);

        return ...parse ID HTML...;
    }

    public Movie FindMovieByTitle(string title)
    {
        var titleUri = ...build URI...;

        var html = _downloader.DownloadHtml(titleUri);

        return ...parse title HTML...;
    }
}

Now, anywhere you need to download movies, you can depend solely on IMovieRepository without being directly coupled to all the implementation details beneath it:

public class NeedsMovies
{
    private readonly IMovieRepository _movies;

    public NeedsMovies(IMovieRepository movies)
    {
        _movies = movies;
    }

    public void DoStuffWithMovie(string title)
    {
        var movie = _movies.FindMovieByTitle(title);

        ...
    }
}

In addition, you can now easily test the parsing logic without having to make web calls. Simply save the HTML and create a downloader which gives it to a repository:

public class TitleHtmlDownloader : IHtmlDownloader
{
    public HtmlDocument DownloadHtml(Uri uri)
    {
        return ...create document from saved HTML...
    }
}

[Test]
public void ParseTitle()
{
    var movies = new MovieRepository(new TitleHtmlDownloader());

    var movie = movies.GetByTitle("The Matrix");

    Assert.AreEqual("The Matrix", movie.Title);

    ...assert other values from the HTML...
}