-2

I have a program which downloads web pages and then scrapes html to create domain specific collection objects e.g. ProductCollection, CatalogCollection, NewsCollection and more. The idea is to create first class collection objects, so these collection classes are encapsulating a list of respective objects. Also, the common reusable logic for these collection objects for scraping is extracted out in a ScrapingCommon class.

public class ProductCollection {
  private readonly List<Product> _products;
  ...
}

The ProductCollection should be instantiated from the data from the scraped web pages. To achieve this I can use factory method pattern. However, the scraping logic is complex enough (divided into one public and multiple private functions) so I don't feel correct to make these methods static. And since this factory method invokes methods from ScrapingCommon either I need to make ScrapingCommon static as well or pass it as a method dependency. Basically, static will spread like infection. If I implement it the factory method way the signatures would look like this:

public static ProductCollection ScrapeProducts(IEnumerable<HtmlDocument> webPages, ScrapingCommon scrapingCommon)

Other possibility is to use make a generic factory which can be instantiated at startup. I came up with following:

public class CollectionFactory {

  private readonly ScrapingCommon _scrapingCommon;

  public T Create<T>(IEnumerable<HtmlDocument> webPages) where T : IScrapableCollection<T> {
    //collection classes have private no arg constructor
    var collectionInstance = (T)Activator.CreateInstance(typeof(T), true);  

    return collectionInstance.Scrape(webPages, _scrapingCommon);
  }

}

public interface IScrapableCollection<T> {
  T Scrape(IEnumerable<HtmlDocument> webPages, ScrapingCommon scrapingCommon);
}

public class ProductCollection : IScrapableCollection<ProductCollection> {

  ... //implementation of Scrape method mutating the state of internal collection
}

//usage
var factory = new CollectionFactory();  //this instance can be injected in any class
var productCollection = factory.Create<ProductCollection>(webPages);

Although this approach works but I don't like it either. It is complex and it is using reflection. Is there a better way to achieve the encapsulation and construction of the collection objects at the same time? Can somebody suggest better alternate ideas?

1 Answer 1

2

Reflection

I don't like it either. It is complex and it is using reflection.

Assuming the generic type being used has a parameterless constructor, you can call new T() without needing to resort to reflection, by putting the related generic constraint on the generic type.

public class MyClass<T> where T : new()
{
    protected T DoStuff()
    {
        var newObject = new T();
    }
}

There is currently no equivalent approach for parametered constructors.


Factories

Factories have two main use cases. In either case, the goal is to give the factory an "expertise" that is necessary for object creation, and which is complex enough to warrant its own class.

The first use case, which I call a "technical" factory, has the factory create the precise type you want, and its expertise lies in knowing how to construct such an object.

public class TechnicalFactory()
{
    public FooService GetFooService()
    {
        return new FooService(
                    new SomeComplexDependency(),
                    new AVeryObscureDependency(),
                    new SecretObject()
        );
    }
}

Note: I omitted a possible IFooService interface as it's not the focal point right now.

The second use case, which I call a "smart" factory, has the factory decide which concrete type to instantiate, and its expertise lies in knowing how choose the right type for the situation.

public class SmartFactory
{
    public IVehicle GetVehicleToTransport(Cargo cargo)
    {
        if(cargo.IsExplosive)
        {
            return new BlastProofTruck(cargo);
        }
        else if(cargo.Weight > 500)
        {
            return new Truck(cargo);
        }
        else if(cargo.Weight < 5)
        {
            return new Bicycle(cargo);
        }
        else
        {
            return new Van(cargo);
        }
    }
}

It is of course possible to combine the two, though for sufficiently complex scenarios it starts turning into a strategy pattern instead of a factory pattern.


Your scenario

Your use case fits with neither use case for a factory. A ProductCollection is not hard to instantiate, nor are you making the factory choose between different concrete types. A factory simply isn't the right fit here, at least not for the way you're going about things right now.

The ProductCollection should be instantiated from the data from the scraped web pages.

I think you've skipped a step there. The ProductCollection shouldn't be instantiated from the data, it should be instantiated from the Products (which, in turn, come from the data).

A ProductFactory makes more sense here than a ProductCollectionFactory. Note that the ProductFactory is still likely to return a list of products rather than a single one (due to the nature of scraping a whole webpage), but the "expertise" of the factory lies in the creation of the product, rather than the list of products, and the factory name should reflect that.

If you look at your current structure, it feels very confused. Every involved class seems to just pass the buck on to the next dependency:

  • The factory, while having a ScrapeProducts method, doesn't actually know how to scrape. It gets passed a ScrapingCommon for that purpose.
  • The factory just creates a new ProductCollection().
  • The ProductCollection, while having a Scrape method, doesn't actually know how to scrape. It gets passed a ScrapingCommon for that purpose.
  • During this entire process, both the webpages and the ScrapingCommon (which presumably will use those webpages as input data) are being passed together, but in separate parameters. They are like two people travelling separately but on the same train, on their way to a date with each other, but they're not starting a conversation with each other until they get to where they said they would meet.

I can only hope that the ProductCollection actually calls the ScrapingCommon and that ScrapingCommon does in fact know how to scrape.

Although you've been light on details, it seems that ProductCollection is the actual ProductFactory here. At least, that is my inference, since you didn't really show that part of the logic.

Instead of juggling these dependencies back and forth, keep things simple:

public class ProductScraper
{
    private readonly ScrapingCommon scrapingCommon;

    public ProductScraper(ScrapingCommon scrapingCommon)
    {
        this.scrapingCommon = scrapingCommon;
    }

    public List<Product> Scrape(HtmlDocument webPage)
    {
        // Whatever the logic is, e.g.:

        var divs = scrapingCommon.GetElementFromPage(webPage, "div");

        var productsOnPage = divs.Select(div =>
                                 new Product()
                                 {
                                     Name = div.InnerText,
                                     IsActive = div.Classes.Contains("active")
                                 });

        return productsOnPage;
    }
}

Some closing notes:

  • Notice how ScrapingCommon and the HtmlDocument do not travel together anymore. The scraping logic is an injected dependency, the document is a method parameter. They start their "conversation" from the moment they meet, much like those people on the same train.
  • I opted to return List<Product> instead of ProductCollection, because your question doesn't show added value for the ProductCollection class. If there are other reasons for it to exist, feel free to change that.
  • I chose to pass a singular webpage as there is no logical benefit from passing a collection of pages all the way down the call stack. Simply call the Scrape method for each page and combine the results.
  • I think ScrapingCommon is either badly named, badly designed, or both. But that's not the focal point right now so I used it for the sake of consistency with your question.
  • ProductScraper can arguably be renamed to ProductFactory, but "factory" is a vague term and I think "scraper" is a bit more precise and appropriate here.
4
  • thankyou for a detailed answer and I am pretty convinced. My current implementation is exactly same as your suggested simple implementation. I wanted to change the implementation to use special Collection classes for some reasons which I will specify in comments. Commented May 3, 2021 at 15:05
  • There are some post processing operations like linking Products with Catalogs. Once we have scraped all products (i.e. the ProductCollection) and all catalogs (i.e. CatalogCollection), I need to link products with catalogs based on the properties of objects. There can be other such post processing operations as well like linking Products with News. Should I create one class each for these post processing operations? I don't want to bloat the ProductScraper class with all kinds of post processing operations. Commented May 3, 2021 at 15:13
  • @NavjotSingh: Different responsibility, different class, different question. Note that I'm not trying to be unhelpful, but it would help for you to separate your concerns more, because stacking multiple things into the same class is going to both overly complicate any answer you're going to get, make communication much harder, and becomes harder to maintain in the future. Factories only care about the creation of their product, and that is where their job description ends.
    – Flater
    Commented May 3, 2021 at 15:17
  • Yeah I get your point....your answer was on point... Commented May 3, 2021 at 15:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.