To read a book thou should have it bought, to buy a book thou should have it found! (Search Abstraction)

I started a hobby project to find the books that I read (paperbacks) so that I can keep track of them (reading start date, finish date, number of pages, etc..)

The first challenge was to find the book on the net, and I realized there were different sources which you can search from, like google books (provides a JSON API), Amazon, isbnsearch.org (isbn search) etc.

I also want to cache the search results in my database so that I can search the books in my database first (to speed things up and not to exhaust API limits).

I then implemented something, and then abstracted something, then implemented and abstracted and ...

Finally, I came up with a generic search abstraction instead of something that searched for books only.

Any suggestions are welcome. I might be missing some pattern that covers all of this. I know this pattern is similar to the abstract factory, but maybe there are ways or patterns that I can use to simplify things more.

Here is the abstraction I finally came up with:

That is, for every different search source I need to implement 3 classes whose responsibilities are very well defined and very very simple. This is the most convenient way that I could achieve supporting modularity and testability.

Here is the source code for the interfaces:

public interface ISearchClient
{
    SourceResponseType ExecuteRemoteSearch(string query);
}

public interface ISearchResponseParser
{
    IEnumerable ParseResponse(SourceResponseType searchResponse);
}

public interface ISearchItemConverter
{
    FinalItemType Convert(SourceItemType sourceItem);
}

And here is one functional abstract class that brings these 3 together:

```
public abstract class SearchProvider
{
protected abstract ISearchClient CreateClient();
protected abstract ISearchResponseParser CreateParser();
protected abstract ISearchItemConverter CreateConverter();

public virtual IEnumerable Search(string query)
{
ISearchClient searchClient = CreateClient();

You said you passed through few iterations of distilling your framework, but did not mention what guided your refactorings.

I would expect most of the effort of such distillation to contribute to the model you use. As I understand, it's now only capable to query by a string, without a capability to query by more specific terms, such as ISBN, Title, Author and date range.

I would suggest to look into Specification Pattern.

Eric Evans talks about this pattern and it's application in the context of a query service in his
Domain-Driven Design: Tackling Complexity in the Heart of Software

Another important target of distillation is removing unnecessary dependencies in the model and the plugin interface. You left the plugin interface out of the scope of this research, but I'd say it's important too.

Dependencies on specialized frameworks such as the Agility Pack seem severe in this light. It's an implementation detail, and i see no reason to bring the types defined in external dependencies to the base classes of the framework:

public IEnumerable ParseResponse(string searchResponse)

In your design you rely on abstract implementations heavily, so your base framework contains useful implementations. It may seem useful, but it actually entangles the design or rather makes it ambiguous.

For example, if some other gateway implementation would need to work over json instead of html, a base class which is coupled to the Agility Pack makes no sense anymore. In this case there are two options: implement another base class similar to your HtmlResponseParser and favor your design, or ignore your design, and implement the other way. In other words, favor Composition over inheritance


To favor composition over inheritance is a design principle that gives
the design higher flexibility, giving business-domain classes and more
stable business domain in the long term. In other words, HAS-A can be
better than an IS-A relationship.

You can still benefit from nice reusable productivity types in form of helper classes, not base classes:

Contracts -> Depends on nothing
Particular Implementation 1 -> Depends on Contracts, Agility Pack
Particular Implementation 2 -> Depends on Contracts, Json.Net

Another issue with this kind of coupling is that in your plugin architecture you make all this irrelevant implementation details visible to the plugin framework.

It makes sense to hide the entire thing behind a non-generic interface:

interface IBookSearchGateway
{
    IEnumerable Query(IBookSpecification specification);
}

As for the factoring of some of the abstractions, it makes sense to make abstractions more powerful without the implementation details leaking into the abstraction:

protected override string SearchUriTemplate
{
    get { return "http://www.isbnsearch.org/isbn/{0}"; }
}

A string format in this case is an implementation detail. You couple your design to the language of string.Format method. This language is not bad in itself, and it would be useful in other contexts, such as User Interface: "Your search returned {0} results from {1} sources"

It would be a more powerful abstraction:

protected override string GetSearchUrl(IBookSpecification spec)
{
    return string.Format("http://www.isbnsearch.org/isbn/{0}", spec.Isbn);
}

As for the new constraint used in generic base implementation, i think it could be too restrictive. I would suggest removing this constraint, and make the factory methods abstract, with the corresponding interface types instead of concrete classes.

This will allow to drop the three last type parameters from the SearchProvider (ClientType, ParserType, ConverterType)

SearchProvider

turns to:

BaseSearchProvider

Search method could be reduced to just one line:

return CreateParser().ParseResponse(CreateClient().ExecuteRemoteSearch(query)).Select(CreateConverter().Convert).ToList();

...which makes me thinking that the algorithm is so simple that it's not worth to create a base class for this sole purpose:

public abstract class BaseSearchProvider
{
    protected abstract ISearchClient CreateClient();
    protected abstract ISearchResponseParser CreateParser();
    protected abstract ISearchItemConverter CreateConverter();

    public virtual IEnumerable Search(string query)
    {
        return CreateParser().ParseResponse(CreateClient().ExecuteRemoteSearch(query)).Select(CreateConverter().Convert).ToList();
    }
}

Again, in favor of composition over inheritance, it's better to encapsulate this algorithm as a Strategy pattern implementation.

As for general code quality, I would suggest using switch construct instead of cascading else if:

switch(pStrongNode.InnerText)
{
    ...
}

I would also note that evaluating pStrongNode.InnerText multiple times could be sub-optimal, but it's already fixed with a switch usage.

I don't know the Agality Pack framework very well, but there are prob

StackExchange Code Review Q#114444, answer score: 3