Besoin de quelques suggestions sur l’architecture de mes logiciels.

Votre API est principalement statique, ce qui signifie que vous vous configurez pour des problèmes de maintenabilité à l’avenir. En effet, les méthodes statiques sont en réalité des singletons, qui présentent des inconvénients importants .

Je suggère de rechercher une approche plus découplée basée sur l’instance. Cela va naturellement séparer la définition de chaque opération de sa mise en œuvre, laissant de la place pour l’extensibilité et la configuration. La facilité d’utilisation d’une API se mesure non seulement à sa surface publique, mais également à son adaptabilité.

Voici comment je voudrais concevoir ce système. Tout d’abord, définissez un élément responsable de la récupération des films:

 public interface IMovieRepository { Movie FindMovieById(ssortingng id); Movie FindMovieByTitle(ssortingng title); } 

Ensuite, définissez un élément responsable du téléchargement de documents HTML:

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

Ensuite, définissez une implémentation de référentiel utilisant un téléchargeur:

 public class MovieRepository : IMovieRepository { private readonly IHtmlDownloader _downloader; public MovieRepository(IHtmlDownloader downloader) { _downloader = downloader; } public Movie FindMovieById(ssortingng id) { var idUri = ...build URI...; var html = _downloader.DownloadHtml(idUri); return ...parse ID HTML...; } public Movie FindMovieByTitle(ssortingng title) { var titleUri = ...build URI...; var html = _downloader.DownloadHtml(titleUri); return ...parse title HTML...; } } 

Désormais, partout où vous avez besoin de télécharger des films, vous pouvez compter uniquement sur IMovieRepository sans être directement couplé à tous les détails de son implémentation:

 public class NeedsMovies { private readonly IMovieRepository _movies; public NeedsMovies(IMovieRepository movies) { _movies = movies; } public void DoStuffWithMovie(ssortingng title) { var movie = _movies.FindMovieByTitle(title); ... } } 

De plus, vous pouvez maintenant facilement tester la logique d’parsing sans avoir à passer d’appels Web. Enregistrez simplement le code HTML et créez un programme de téléchargement qui le transmettra à un référentiel:

 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 Masortingx"); Assert.AreEqual("The Masortingx", movie.Title); ...assert other values from the HTML... } 

Voici quelques suggestions, rien de majeur, juste quelques points à considérer.

1. Je comprends que vous souhaitiez garder l’API minimale, rendant ainsi l’parsingur et le téléchargeur privés / internes, mais vous voudrez peut-être envisager de les rendre publics de toute façon. La raison principale est que, puisqu’il s’agira d’un projet open source, il est fort probable que vous obtiendrez des gens qui sont des pirates informatiques. Si, par hasard, ils souhaitent faire quelque chose qui n’est pas directement pris en charge par l’API que vous fournissez, ils apprécieront que vous leur laissiez le temps de le faire eux-mêmes. Rendez les cas d’utilisation «standard» aussi simples que possible, mais aidez également les utilisateurs à en faire ce qu’ils veulent.

2. Il semble qu’il y ait une duplication de données entre votre classe Movie et votre parsingur. Plus précisément, l’parsingur obtient les champs définis par votre film. Il semble plus logique de faire de Movie un object de données (uniquement les propriétés) et de faire en sorte que la classe Parser l’exploite directement. Ainsi, votre parsingur FromMovieTitle pourrait renvoyer un film au lieu d’un parsingur. Maintenant, cela soulève la question de savoir quoi faire avec les méthodes de la classe Movie FindMovie et FindKnownMovie . Je dirais que vous pouvez créer une classe MovieFinder ces méthodes et utiliser l’parsingur pour renvoyer un film.

3. Il semble que les tâches d’parsing pourraient devenir assez complexes car vous allez gratter du HTML (du moins sur la base des commentaires). Vous pouvez envisager d’utiliser un modèle de chaîne ou de responsabilité (ou quelque chose de similaire) dans l’parsingur avec une interface simple qui vous permettrait de créer une nouvelle implémentation pour les différents éléments de données que vous souhaitez extraire. Cela garderait la classe Parser assez simple et permettrait également à d’autres personnes d’étendre plus facilement le code pour extraire des éléments de données que vous ne supporteriez peut-être pas directement (encore une fois, puisqu’il s’agit de personnes Open Source qui ont tendance à aimer l’extensibilité facile).

En règle générale, si vous gardez à l’esprit le principe de responsabilité unique et le principe d’ ouverture / fermeture ainsi que votre objective de simplifier l’utilisation standard, vous devriez obtenir quelque chose que les gens trouveront facile à utiliser pour les éléments que vous pensiez soutenir, et facile à étendre pour les choses que vous n’avez pas.

Je n’exposerais que les éléments qui ont un sens à exposer. Pour vous coder, le résultat final est une information de film. Le téléchargeur et l’parsingur sont inutiles, sauf s’ils sont utilisés pour obtenir les informations sur le film. Il n’y a donc aucune raison de les exposer.

De plus, dans votre classe de film, je ne ferais que rendre les informations Getable, et non pas configurables aussi. il n’y a pas de fonctionnalité “enregistrer” dans la classe, il n’y a donc aucune raison de modifier les informations une fois que vous les avez obtenues.

Autre que cela si cela est pour d’autres personnes, je voudrais commenter ce que chaque classe, et membre et chaque variable de classe public / privé sont pour. Pour la classe Movie, j’inclurais probablement un exemple d’utilisation dans le commentaire de la classe.

La dernière chose, s’il y a une erreur dans les deux classes privées, l’utilisateur de la classe Movie doit être informé d’une manière ou d’une autre. Peut-être une variable publique bool appelée succès?

Sur une note de préférence personnelle, pour votre classe de film, je voudrais que vos deux fonctions soient des constructeurs afin que je puisse juste construire la classe comme suit.

Movie myMovie = new Movie (“Nom”); ou Movie myMovie = new Movie (1245);

Tout d’abord, j’estime que votre principale préoccupation est erronée. D’après mon expérience, la conception d’une architecture pour la “facilité d’utilisation”, bien que jolie à regarder avec toutes ses fonctionnalités encapsulées, a tendance à être hautement interdépendante et rigide. Au fur et à mesure que l’application reposant sur ce principe grandit, vous rencontrerez de graves problèmes de dépendances (les classes finissent par devenir directement dépendantes de plus en plus, et indirectement par dépendre de tout ce qui se trouve dans votre système). Nain les avantages de “facilité d’utilisation” que vous pourriez gagner.

La séparation des préoccupations et la responsabilité unique sont deux des règles les plus importantes de l’architecture. Ces deux règles dictent des choses telles que maintenir les préoccupations d’infrastructure (access aux données, parsing) séparées des préoccupations d’entreprise (trouver des films) et de s’assurer que chaque classe que vous écrivez n’est responsable que d’une chose (représenter les informations sur le film, rechercher des films individuels.)

Votre architecture, même si elle est actuellement petite, a déjà violé la responsabilité unique. Votre classe Movie, bien qu’elle soit élégante, cohérente et facile à utiliser, associe deux responsabilités: représenter les informations sur le film et gérer les recherches de film. Ces deux responsabilités devraient être dans des classes séparées:

 // Data Contract (or Data Transfer Object) public class Movie { public Image Poster { get; set; } public ssortingng Title { get; set; } public DateTime ReleaseDate { get; set; } public ssortingng Rating { get; set; } public ssortingng Director { get; set; } public List Writers { get; set; } public List Genres { get; set; } public ssortingng Tagline { get; set; } public ssortingng Plot { get; set; } public List Cast { get; set; } public ssortingng Runtime { get; set; } public ssortingng Country { get; set; } public ssortingng Language { get; set; } } // Movie database searching service contract public interface IMovieSearchService { Movie FindMovie(ssortingng Title); Movie FindKnownMovie(ssortingng ID); } // Movie database searching service public partial class MovieSearchService: IMovieSearchService { public Movie FindMovie(ssortingng 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(ssortingng 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. } } 

Cela peut sembler sortingvial, mais séparer le comportement de vos données peut devenir critique à mesure que le système se développe. En créant une interface pour votre service de recherche de films, vous offrez découplage et flexibilité. Si, pour une raison quelconque, vous devez append un autre type de service de recherche de film offrant les mêmes fonctionnalités, vous pouvez le faire sans mettre en péril votre clientèle. Le type de données de film peut être réutilisé, vos clients se lient à l’interface IMovieSearchService plutôt qu’à une classe concrète, ce qui permet d’échanger les implémentations (ou plusieurs implémentations simultanément). projet que la classe MovieSearchService.

Vous avez pris la bonne décision en écrivant la classe d’parsingur et en séparant l’parsing de la fonctionnalité de recherche de film. Cela répond à la règle de la séparation des préoccupations. Cependant, votre approche va entraîner des difficultés. D’une part, il est basé sur des méthodes statiques, qui sont très inflexibles. Chaque fois que vous devez append un nouveau type d’parsingur, vous devez append une nouvelle méthode statique et mettre à jour le code nécessaire à l’utilisation de ce type d’parsing. Une meilleure approche consiste à utiliser la puissance du polymorphism et du static de fossé:

 public abstract class Parser { public abstract IEnumerable Parse(ssortingng criteria); } public class ByTitleParser: Parser { public override IEnumerable Parse(ssortingng 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 Parse(ssortingng 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 Parse(ssortingng 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 } } 

Enfin, un autre principe utile est l’dependency injection. Plutôt que de créer directement de nouvelles instances de vos dépendances, résumez leur création via quelque chose qui ressemble à une usine et injectez vos dépendances et usines dans les services qui en ont besoin:

 public class ParserFactory { public virtual Parser GetParser(ssortingng 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(ssortingng 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(ssortingng 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; } } 

Cette version améliorée présente plusieurs avantages. D’une part, il n’est pas responsable de la création d’instances de ParserFactory. Cela permet d’utiliser plusieurs implémentations de ParserFactory. Au début, vous ne pouvez rechercher que sur IMDB. À l’avenir, vous voudrez peut-être effectuer une recherche sur d’autres sites. Vous pouvez également fournir d’autres parsingurs pour une implémentation alternative de l’interface IMovieSearchService.