需要一些关于我的软件架构的建议。 [代码审查]

Question

我正在制作一个开源 C# 库供其他开发人员使用。我最关心的是易用性。这意味着使用直观的名称、直观的方法用法等。

这是我第一次与其他人一起做某事，所以我真的很关心架构的质量。另外，我不介意学习一两件事。 :)

我有三个课程： 下载器、解析器和影片

我在想最好只公开我的库的 Movie 类，并让 Downloader 和 Parser 保持隐藏，不被调用。

最终，我看到我的库被这样使用了。

使用 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.
}

您能否回顾一下我的计划，并指出我做出的任何错误判断以及我可以改进的地方。

记住，我主要关心的是易用性。

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        
    }
}

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

你们认为我正在走向一条好的道路，还是我在为以后的伤害世界做好准备？

我最初的想法是将下载、解析和实际填充分开，以便轻松拥有一个可扩展的库。想象一下，如果有一天网站更改了它的 HTML，那么我只需要修改解析类，而无需触及 Downloader.cs 或 Movie.cs 类。

感谢您的阅读和帮助！

还有其他想法吗？

Answer 1

我只会公开有意义的项目。对于您的代码，最终结果是电影信息。下载器和解析器是无用的，除非用于获取电影信息，因此没有理由公开它们。

在你的电影类中，我只会将信息设为 Getable，而不是 Setable。该类没有“保存”功能，因此一旦获得信息就没有理由编辑信息。

除此之外，如果这是针对其他人的，我会评论每个类、成员和每个公共/私有类变量的用途。对于 Movie 类，我可能会在类注释中包含一个如何使用它的示例。

最后，如果两个私有类出现错误，需要以某种方式通知 Movie 类的用户。可能是一个名为 success 的公共 bool 变量？

根据个人喜好，对于 Your Movie 类，我会让你的两个函数成为构造函数，这样我就可以按如下方式构建类。

电影 myMovie = new Movie("Name"); 要么 电影 myMovie = new Movie(1245);

Answer 2

这里有一些建议，没什么大不了的，只是一些需要考虑的事情。

1. 我了解您希望保持 API 最小化，从而将 Parser 和 Downloader 设为私有/内部，但您可能仍需要考虑将它们设为公开。最大的原因是，由于这将是一个开源项目，因此您很可能会遇到黑客。如果碰巧他们想做一些你提供的 API 不直接支持的事情，他们会很感激你让他们可以自己做这些事情。使“标准”用例尽可能简单，但也让人们可以轻松地做任何他们想做的事情。

2. 您的 Movie 类和 Parser 之间似乎存在一些数据重复。具体来说，解析器正在获取您的电影定义的字段。使 Movie 成为数据对象（只是属性）并让 Parser 类直接对其进行操作似乎更有意义。所以你的解析器FromMovieTitle 可以返回一个电影而不是一个解析器。现在提出了如何处理 Movie 类 FindMovie 和 FindKnownMovie 上的方法的问题。我想说你可以创建一个包含这些方法的MovieFinder 类，然后他们会利用解析器返回一个电影。

3. 看起来解析任务可能会变得相当复杂，因为您将要抓取 HTML（至少基于 cmets）。您可能需要考虑在解析器中使用Chain or Responsibility 模式（或类似的东西）和一个简单的接口，允许您为要提取的各种数据元素创建一个新的实现。这将使 Parser 类保持相当简单，并且还允许其他人更轻松地扩展代码以提取您可能不直接支持的数据元素（再次强调，因为这是开源的人们往往喜欢易于扩展）。

一般来说，如果您牢记Single Responsibility Principle 和Open/Closed Principle 以及保持标准使用简单的目标，那么您最终应该会得到人们会发现易于使用的东西支持，并且易于扩展您没有的东西。

Answer 3

您的 API 大多是静态的，这意味着您要为将来的可维护性问题做好准备。这是因为静态方法实际上是单例的，which have some significant drawbacks。

我建议争取一种更加基于实例、解耦的方法。这自然会将每个操作的定义与其实现分开，为可扩展性和配置留出空间。 API 的易用性不仅取决于其公共表面，还取决于其适应性。

下面是我设计这个系统的方法。首先，定义一些负责获取电影的东西：

public interface IMovieRepository
{
    Movie FindMovieById(string id);

    Movie FindMovieByTitle(string title);
}

接下来，定义负责下载 HTML 文档的东西：

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

然后，定义一个使用下载器的存储库实现：

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...;
    }
}

现在，您可以在任何需要下载电影的地方单独依赖IMovieRepository，而无需直接耦合到它下面的所有实现细节：

public class NeedsMovies
{
    private readonly IMovieRepository _movies;

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

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

        ...
    }
}

此外，您现在无需进行网络调用即可轻松测试解析逻辑。只需保存 HTML 并创建一个下载器，将其提供给存储库：

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...
}

Answer 4

嗯，首先，我认为您的主要关注点被误导了。以我的经验，设计一个“易于使用”的架构虽然看起来很漂亮，但它们的所有封装功能往往是高度相互依赖和僵化的。随着建立在这样一个主体上的应用程序的增长，您将遇到严重的依赖问题（类最终变得越来越直接依赖，并最终间接依赖于系统中的所有内容。）这会导致真正的维护噩梦，让您可能获得的“易用性”优势相形见绌。

架构中最重要的两条规则是Separation of Concerns 和Single Responsibility。这两条规则规定了一些事情，例如将基础设施问题（数据访问、解析）与业务问题（查找电影）分开，并确保您编写的每个类只负责一件事（表示电影信息、搜索单个电影）。

您的架构虽然目前很小，但已经违反了单一职责。您的 Movie 类虽然优雅、有凝聚力且易于使用，但它融合了两种职责：表示电影信息和服务电影搜索。这两个职责应该在不同的类中：

// Data Contract (or Data Transfer Object)
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; }
}

// Movie database searching service contract
public interface IMovieSearchService    
{
        Movie FindMovie(string Title);
        Movie FindKnownMovie(string ID);
}

// Movie database searching service
public partial class MovieSearchService: IMovieSearchService
{
        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.
        }
}

这可能看起来微不足道，但是随着系统的增长，将行为与数据分开可能变得至关重要。通过为您的电影搜索服务创建接口，您可以提供解耦和灵活性。如果您出于某种原因需要添加提供相同功能的另一种类型的电影搜索服务，您可以这样做而不会破坏您的消费者。 Movie 数据类型可以重用，您的客户端绑定到 IMovieSearchService 接口而不是具体类，从而允许互换实现（或同时使用多个实现）。最好将 IMovieSearchService 接口和 Movie 数据类型放在单独的项目比 MovieSearchService 类。

您编写了解析器类，并将解析与电影搜索功能分开，这是一个很好的举措。这符合关注点分离规则。但是，您的方法将导致困难。一方面，它基于静态方法，非常不灵活。每次需要添加新类型的解析器时，都必须添加新的静态方法，并更新任何需要使用该特定解析类型的代码。更好的方法是利用多态性的力量，抛弃静态：

public abstract class Parser
{
    public abstract IEnumerable<Movie> Parse(string criteria);
}

public class ByTitleParser: Parser
{
    public override IEnumerable<Movie> Parse(string title)
    {
        // TODO: Logic to parse movie information by title
        // Likely to return one movie most of the time, but some movies from different eras may have the same title
    }
}

public class ByActorParser: Parser
{
    public override IEnumerable<Movie> Parse(string actor)
    {
        // TODO: Logic to parse movie information by actor
        // This one can return more than one movie, as an actor may act in more than one movie
    }
}

public class ByIdParser: Parser
{
    public override IEnumerable<Movie> Parse(string id)
    {
        // TODO: Logic to parse movie information by id
        // This one should only ever return a set of one movie, since it is by a unique key
    }
}

最后，另一个有用的原则是依赖注入。与其直接创建依赖项的新实例，不如通过工厂之类的东西抽象它们的创建，并将你的依赖项和工厂注入到需要它们的服务中：

public class ParserFactory
{
    public virtual Parser GetParser(string criteriaType)
    {
        if (criteriaType == "bytitle") return new ByTitleParser();
        else if (criteriaType == "byid") return new ByIdParser();
        else throw new ArgumentException("Unknown criteria type.", "criteriaType");
    }
}

// Improved movie database search service
public class MovieSearchService: IMovieSearchService
{
        public MovieSearchService(ParserFactory parserFactory)
        {
            m_parserFactory = parserFactory;
        }

        private readonly ParserFactory m_parserFactory;

        public Movie FindMovie(string Title)
        {
            var parser = m_parserFactory.GetParser("bytitle");
            var movies = parser.Parse(Title); // Parse method creates an enumerable set of Movies that matched "Title"

            var firstMatchingMovie = movies.FirstOrDefault();

            return firstMatchingMovie;
        }

        public Movie FindKnownMovie(string ID)
        {
            var parser = m_parserFactory.GetParser("byid");
            var movies = parser.Parse(Title); // Parse method creates an enumerable set of Movies that matched "ID"

            var firstMatchingMovie = movies.FirstOrDefault();

            return firstMatchingMovie;
        }
}

这个改进的版本有几个好处。一方面，它不负责创建 ParserFactory 的实例。这允许使用 ParserFactory 的多个实现。早期，您只能搜索 IMDB。将来，您可能希望搜索其他站点，并且可以提供用于 IMovieSearchService 接口的替代实现的替代解析器。