Je fais une bibliothèque C # Open Source pour d'autres développeurs à utiliser. Ma principale préoccupation est C'est la première fois que j'ai fait quelque chose avec d'autres personnes à l'esprit, je suis donc vraiment préoccupé par la qualité de l'architecture. De plus, cela ne me dérangerait pas d'apprendre une chose ou deux. :) p> J'ai trois classes:
Downloader, analyseur et film strong> P> Je pensais qu'il serait préférable de ne pas exposer la classe de film de ma bibliothèque et que le téléchargeur et l'analyseur restent cachés à l'invocation. P> En fin de compte, je vois que ma bibliothèque étant utilisée comme celle-ci. p> Utiliser freeimdb; p> pouvez-vous revoir comment je planifie ce que je planifie ce que je planifie ce que je planifie ce Signifiez tout mauvais jugement que j'ai fait et où je pourrais m'améliorer. P> N'oubliez pas que ma principale préoccupation est la facilité d'utilisation. EM> P> parser.cs strong> p> Est-ce que vous pensez que je pense que je me dirigeai vers un bon sentier, ou je me suis montant pour un monde de blessé plus tard? P> Ma pensée originale était de séparer le téléchargement, l'analyse et le peuplement effectif avoir facilement une bibliothèque extensible. Imaginez si un jour, le site Web a changé de HTML, je ne devrais alors que modifier la classe d'analyse sans toucher le téléchargeur.cs ou la classe de film.cs. P> Merci de la lecture et de l'aide! P> --- ------------------------------------------------- h2>
4 Réponses :
J'exposerais uniquement les éléments qui ont du sens à exposer. Pour votre code, le résultat final est des informations sur le film. Le téléchargeur et l'analyseur sont inutiles que si l'on utilise les informations sur le film afin qu'il n'y ait aucune raison de les exposer. P>
Également dans votre classe de cinéma, je ne ferais que les informations pouvant être gênantes, pas aussi. Il n'y a pas de fonctionnalité "Enregistrer" à la classe, il n'ya donc aucune raison de modifier les informations une fois que vous l'obtenez. P>
Autre que ceci est pour d'autres personnes, je commenterais ce que chaque classe, membre et chaque variable de classe publique / privée sont destinés. Pour la classe de cinéma, j'inclurais probablement un exemple de la façon de l'utiliser dans le commentaire de la classe. p>
La dernière chose, s'il y a une erreur dans les deux classes privées, l'utilisateur de la classe de film doit être informé d'une manière ou d'une autre. Peut-être une variable de bool publique appelée succès? P>
sur une note de préférence personnelle, pour votre classe de film, je voudrais que vos deux fonctions soient constructeurs afin que je puisse simplement construire la classe comme suit. P>
film mymovie = nouveau film ("nom"); ou Film mymovie = nouveau film (1245); p>
Voici quelques suggestions, rien de majeur, juste certaines choses à considérer. p>
Je comprends que vous vouliez garder l'API minimal, rendant ainsi l'analyseur et le téléchargeur privé / interne, mais vous voudrez peut-être envisager de les rendre publics de toute façon. La plus grande raison est que, puisque cela va être un projet open source, vous allez probablement obtenir des personnes qui sont, bien, des pirates informatiques. Si, par hasard, ils veulent faire quelque chose qui n'est pas directement soutenu par l'API que vous fournissez, ils apprécieront que vous rendrez les bits à leur disposition pour le faire eux-mêmes. Faites les cas «standard» d'utilisation aussi simples que possible, mais permettez également aux gens de faire ce qu'ils veulent avec elle. P> LI>
On dirait qu'il existe une duplication de données entre votre classe vidéo et votre analyseur. Plus précisément, l'analyseur reçoit les champs définis par votre film. Il semble que cela aurait plus de sens de faire de film un objet de données (uniquement des propriétés) et de faire fonctionner directement la classe d'analyseur. Donc, votre parser En règle générale Si vous conservez le Principe de responsabilité unique et PRINCIPE OUVERT / FERMÉ À l'esprit avec votre objectif de conserver l'utilisation standard facile, vous devriez vous retrouver avec quelque chose que les gens trouvera facile à utiliser pour les choses que vous avez pensées à soutenir et à prolonger facilement les choses que vous n'avez pas faites. P> demovietitle code> pourrait renvoyer un film au lieu d'un analyseur. Maintenant, cela soulève la question de savoir quoi faire avec les méthodes de la classe de films FindMovie code> et Trouvernownmovie code>. Je dirais que vous pouvez créer une classe Moviefinder code> qui y avait ces méthodes et utilise l'analyseur pour renvoyer un film. P> LI>
Votre API est principalement statique, ce qui signifie que vous vous préparez à des problèmes de maintenabilité à l'avenir. En effet, les méthodes statiques sont en fait des singletons, qui ont des inconvénients significatifs a >. Je suggère de m'efforcer d'une approche plus basée sur une instance, découplée. Cela séparera naturellement la définition de chaque opération de sa mise en œuvre, laissant la place à l'extensibilité et à la configuration. La facilité d'utilisation d'une API est mesurée non seulement par sa surface publique, mais également par sa adaptabilité. P> Voici comment je voudrais concevoir ce système. Tout d'abord, définissez quelque chose qui est responsable de la récupération de films: P> 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...
}
@RCIX: Mon objectif n'est pas simplement de déplacer correctement les bits; Il est également de minimiser l'impact du changement. Les systèmes écrits dans ce style (inversion de dépendance / CIO) sont résilients et évoluent gracieusement. Les API statiques / étroitement couplées ne le font généralement pas, en particulier lorsqu'elles sont dépendantes d'un service tiers. Une bibliothèque pour d'autres développeurs mérite une conception durable.
Vous pouvez également vouloir faire une étape plus loin et créer un iMoviedownloader code> et iMovieParser code> afin que vous puissiez tirer parti de la capacité d'analyser d'autres types de documents, de dire XML ou un service Web.
Eh bien, tout d'abord, je pense que votre principale préoccupation est erronée. Dans mon expérience, concevoir une architecture pour "facilité d'utilisation", tandis que jolie à regarder avec toutes leurs fonctionnalités encapsulées, tendent à être très interdépendante et rigide. En tant qu'application construite sur une telle principale pousse, vous rencontrerez de graves problèmes avec des dépendances (les classes finissent directement de plus en plus, de plus en plus, et indirectement dépendantes, finalement, tout dans votre système.) Cela conduit à de véritables cauchemars de maintenance qui nain La "facilité d'utilisation" des avantages que vous pourriez gagner.
Deux des règles d'architecture les plus importantes sont Séparation des préoccupations et Responsabilité unique . Ces deux règles dictent des choses comme la préoccupation des infrastructures (accès des données, analyse) séparées des préoccupations commerciales (trouver des films) et en veillant à ce que chaque classe que vous écrivez n'est responsable d'une chose (représentant des informations sur le film, à la recherche de films individuels.) P>
Votre architecture, tandis que c'est actuellement petite, a déjà violé la responsabilité unique. Votre classe de cinéma, bien qu'elle soit élégante, cohérente et facile à utiliser, mélange deux responsabilités: représentant des informations sur les films et servir des recherches de films. Ces deux responsabilités devraient être dans des classes séparées: p> Cela peut sembler trivial, cependant, séparer le comportement de vos données peut devenir essentiel en tant que système grandissant. En créant une interface pour votre service de recherche de film, vous fournissez un découplage et une flexibilité. Si vous avez besoin d'ajouter un autre type de service de recherche de film qui fournit la même fonctionnalité, vous pouvez le faire sans casser vos consommateurs. Le type de données de film peut être réutilisé, vos clients se lient à l'interface IMOVIESEARSERVICE plutôt qu'une classe de béton, permettant aux implémentations d'être interchangées (ou de multiples implémentations utilisées simultanément.) Il est préférable de mettre le type d'interface IMOVIESEarchService et de type de données de film dans une Projet que la classe FunsearchService. P> Vous avez fait un bon geste en écrivant la classe d'analyse et en gardant une analyse séparée de la fonctionnalité de recherche de films. Cela répond à la règle de séparation des préoccupations. Cependant, votre approche va conduire à des difficultés. Pour un, il est basé sur des méthodes statiques, qui sont très inflexibles. Chaque fois que vous devez ajouter un nouveau type d'analyseur, vous devez ajouter une nouvelle méthode statique et mettre à jour l'un des codes qui doit utiliser ce type d'analyse particulier. Une meilleure approche est d'utiliser la puissance du polymorphisme et du fossé statique: p> enfin, une autre principale utile est une injection de dépendance. Plutôt que de créer de nouvelles instances de vos dépendances, résumez leur création via quelque chose comme une usine et injecter vos dépendances et vos usines dans les services qui en ont besoin: p> cette version améliorée a plusieurs avantages. Pour un, il n'est pas responsable de la création d'instances de la facalisation. Cela permet d'utiliser de multiples implémentations de l'analyse analysé. Tôt, vous ne pouvez rechercher que IMDB. À l'avenir, vous souhaiterez peut-être rechercher d'autres sites et des analyseurs alternatifs pour des implémentations alternatives de l'interface IMOVIESEarchService peuvent être fournies. P> P>
Ça a l'air bien, votre analyseur n'utilise rien et puis pourrait être trompeur. L'analyseur est vraiment un DAO (objet d'accès aux données) Je nommerais IT Moviedao et votre film est une entité (vous pouvez laisser cela seul)
L'analyseur en ce moment est sans aucun code d'analyse réel. Il est à but de recevoir un document HTML, puis de gratter toutes les informations pertinentes et de l'enregistrer sur un objet de film instancié. Le titre (), affiche (), etc. Les méthodes sont juste là pour rebondir les champs de l'objet film créé car il s'agissait de gratter.
Ensuite, j'aurais un film, Moviedao, MovieParse et un contrôleur qui gère l'interaction entre tous. Peut-être passer dans l'analyseur à la DAO sur la construction ou la récupération comme argument.
Je conviens également que la Findmovie et la FindnownMovie devraient être une surcharge et être différenciée par des paramètres. Ces méthodes doivent être déplacées vers la DAO (c'est l'objetc responsable de la récupération de films.