Web scraper for e-commerce sites

Question

I'm building web scraper application which takes name, code and price from few sites. I thought factory pattern would fit in my application. I would like to someone review my code and tell if I something missed out.

I have class Item which holds scraped data.

public class Item
{
    public string Code { get; set; }

    public string Name { get; set; }

    public string Price { get; set; }
}

Interface which have method RunScrapingAsync with one parameter list of item codes which I need scape.

public interface IWebScraper
{
    Task<List<Item>> RunScrapingAsync(List<string> itemCodes);
}

There I have implementations for three scrapers (Amazon, EBay, AliExpress):

public class AmazonWebScraper : IWebScraper
{
    private static HttpClient client;
    public List<string> ItemCodes { get; set; }

    public AmazonWebScraper()
    {
        client = new HttpClient(new HttpClientHandler() { Proxy = null });
        client.BaseAddress = new Uri("https://amazon.com");
    }

    public async Task<List<Item>> RunScrapingAsync(List<string> itemCodes)
    {
         ItemCodes = itemCodes;
         ConcurrentBag<Item> itemsConcurrentBag = new ConcurrentBag<Item>();
         //for simplicity this logic is not important no need to go in details
         return itemsConcurrentBag.ToList();
    }
}

public class EBayWebScraper : IWebScraper
{
    private static HttpClient client;
    public List<string> ItemCodes { get; set; }

    public EBayWebScraper()
    {
        client = new HttpClient(new HttpClientHandler() { Proxy = null });
        client.BaseAddress = new Uri("https://ebay.com");
    }

    public async Task<List<Item>> RunScrapingAsync(List<string> itemCodes)
    {
         ItemCodes = itemCodes;
         ConcurrentBag<Item> itemsConcurrentBag = new ConcurrentBag<Item>();
         //for simplicity this logic is not important no need to go in details
         return itemsConcurrentBag.ToList();
    }
}

public class AliExpressWebScraper : IWebScraper
{
    private static HttpClient client;
    public List<string> ItemCodes { get; set; }

    public AliExpressWebScraper()
    {
        client = new HttpClient(new HttpClientHandler() { Proxy = null });
        client.BaseAddress = new Uri("https://aliexpress.com");
    }

    public async Task<List<Item>> RunScrapingAsync(List<string> itemCodes)
    {
         ItemCodes = itemCodes;
         ConcurrentBag<Item> itemsConcurrentBag = new ConcurrentBag<Item>();
         //for simplicity this logic is not important no need to go in details
         return itemsConcurrentBag.ToList();
    }
}

Here is my factory class WebScraperFactory:

public enum WebSite
{
     Amazon,
     EBay,
     AliExpress
}

public class WebScraperFactory
{
    private readonly Dictionary<WebSite, IWebScraper> _scrapers;

    public WebScraperFactory()
    {
        _scrapers = new Dictionary<WebSite, IWebScraper>();

        foreach (WebSite webSite in Enum.GetValues(typeof(WebSite)))
        {
            _scrapers.Add(webSite, (IWebScraper)Activator.CreateInstance(Type.GetType($"{webSite}WebScraper")));
        }
    }

    public async Task<List<Item>> Execute(WebSite website, List<string> itemCodes) => await _scrapers[website].RunScrapingAsync(itemCodes);
}

This is WinForm app so user have option to run one or more scraping (they are not all mandatory to run). So if user choose to run Amazon and AliExpress it will choose two files with codes add it in Dictionary and on every chosen website call webscraper factory.

Example:

var websitesItemCodes = new Dictionary<WebSite, List<string>>
{
    {WebSite.Amazon, amazonCodes},
    {WebSite.AliExpress, aliExpressCodes}
}

var websitesItems = new Dictionary<WebSite, List<Item>>
{
    {WebSite.Amazon, null},
    {WebSite.AliExpress, null}
}

var factory = new WebScraperFactory();

foreach(var webSite in websitesItemCodes.Keys)
{
    websitesItems[webSite] = await factory.Execute(webSite, websitesItemCodes[webSite]);
}

Answer 1

The point of a factory is to separate objects from their construction. A factory becomes compelling when that construction is somewhat complex. That's all a factory should be doing.

This factory violates Single Responsibility. It keeps object references and implements a common method for itself. That's the job of a base class, either "concrete" or abstract. This lazy construct will bite you in a growing code base. Hear me now and believe me later!

We did that once - no, many times in a factory that generated a number of types. Inevitably a required change on one class took three months because we had to decouple from the factory. This necessity was not trivial and it dominoed to others. Professionally embarrassing, the damaged reputation is hard to recover.

---

These classes should be subclasses because that is what they obviously are. An abstract class as the base-class is appropriate in this case. An "interface" - the C# keyword interface kind - is appropriate when we want to give unrelated classes the same behavior. By the way, using both interface and abstract class to define the same behavior is superfluous, and in my opinion wrong. It can lead to design corruption and problematic code variations. I've experienced this also.

---

Keep the factory but get the objects decoupled.

---

---

IWebScraper as Design

Web Scraping is a complex endeavor using many parts, participating classes,and algorithm details. This IWebScraper interface is obviously insufficient as it assumes away all complexity and details.

Give me a web scraping code framework, let's call it - which you have done. The framework's API is sufficient to implement web scrapers and therefore IWebScraper interface is unnecessary.

The web scraper "framework" code:

• provides an API to code against
• requires implementation where needed
• provides customization within temporal execution - method template pattern
• fully consistent with the maxim Code to interfaces not implementation
• Protects design integrity
• Ensures execution consistency across implementations