So I decided to expand a little one-off C# console program I wrote awhile ago to download the free e-book offered by Syncfusion, and decided to also do it for other free e-book publishers... like O'Reilly.

But I now want to make it a generic e-book downloader, and make it easily expandable and maintainable. I plan to open source it on Github, and accept pull requests for any additional free e-book sites.

I would like input on how to best follow and implement SOLID principles for the downloader classes.

So far I have: An interface to define the most basic behavior/functionality

public interface IEbookDownloader
{
void Run();
void OutputLogs();
}


public class BaseDownloader : IEbookDownloader
{
public Uri EbooksUrl { get; private set; }

public bool OvewriteExisting { get; private set; }

/// <summary>
/// </summary>
{
this.OvewriteExisting = overwrite;

this.EbooksUrl = Uri.IsWellFormedUriString(ebooksurl, UriKind.Absolute) ? new Uri(ebooksurl) : null;

}

public void OutputLogs()
{
}

{
// check data make sure we have proper dl destination and dl source url
{
}

if (this.EbooksUrl == null)
{
throw new Exception("The url passed is malformed.");
}

if (!File.Exists(ebookspagehtmlpath) || this.OvewriteExisting)
{
using (var wclient = new WebClient())
{
}
}

// TODO: For this generic one just web-scrape parse/regex html for urls that have: href='anything.[epub|pdf|mobi|zip]'
var matches = Regex.Matches(pagehtml, @"href='(.+\.[epub|pdf|mobi|zip])'", RegexOptions.IgnoreCase);
if (matches.Count > 0)
{
foreach (Match matchgroup in matches)
{
string titleslug = string.Empty;
var cg = matchgroup.Groups[1].Value;
if (Uri.IsWellFormedUriString(cg, UriKind.Absolute))
{
}
}
}
}

public void Run()
{

// thingy needed for TitleCasing Ebook folder names
var textinfo = new CultureInfo("en-US", false).TextInfo;

using (var wclient = new WebClient())
{
var ok200count = 0;
{

var dotindex = filename.LastIndexOf(".");
var booktitle = textinfo.ToTitleCase(filename.Substring(0, dotindex)).Replace(" ", "_");

if (!Directory.Exists(ebookdir))
{
Directory.CreateDirectory(ebookdir);
}

{
var ebookfilepath = Path.Combine(ebookdir, filename);
if (!File.Exists(ebookfilepath) || this.OvewriteExisting)
{
}
else
{
Console.WriteLine("Skipped: File already in {0}", ebookfilepath);
}

}
catch (WebException e)
{
Console.WriteLine("Error! Could not find {0} at {1}", booktitle, link);
Console.Write("Client error was: {0}", e.Message);
continue;
}
}

}
}
}


using System;

{
{
}

public override void Run()
{
// specific steps for Syncfusion's site
}

}

• Parse HTML with regex... this answer is also useful ;-) – Phrancis Feb 27 '17 at 5:25
• haha yeah thanks... I've realized every question with regex + html leads to that answer! Though I'm trying to focus more how to properly structure this program... Do you recommend any good C# html parser libs? Is there any built-in ones? I've run across TidyNet and a few others but I'm not sure which to choose – unknownprotocol Feb 27 '17 at 7:33
• I don't really do much C#, so I wouldn't know. I've read that XML parsers work good for [X]HTML, so you could use the built-in XmlReader perhaps. – Phrancis Feb 28 '17 at 0:34

var downloader = new SyncfusionDownloader();


public interface IEbookDownloader
{
}


It doesn't need to do anything else. Logging should be an internal matter of the downloader.

Next, if you want to provide a base implementation for such a downloader then it should be an abstract class and the name should be like the interface but without the I.

public abstract class EbookDownloader : IEbookDownloader
{
...
}


It makes no sense to instantiate it and use it on its own because it cannot download anything yet or rather it shouldn't. The derived SyncfusionDownloader overrides the base's implementation so why is it implemented at all? If the base-downloader is able to download and can be used on its own then give it some concrete name and don't use it as a base class for other downloaders that most likely will override its methods anyway.

# SRP

A downloader should only be able to download. If you need to search for urls or parse some websites then you need other classes that specialize in such tasks. Don't put all these responsibilities insinde a downloader. It's not its job.

It might even turn out that you will have only one downloader but mutliple website-parsers or link finders etc.

# Logging

The user of the downloader should not need to call the log methods. Logging should be invisible and ideally you should use dependency injection to pass the logger to the downloader. Currently it's not only tightly coupled to the console but also to the file system and even hardcoded file names. This is all bad for testing.

• Got it, thanks a lot. Yeah I think I need to break up the "search for ebook urls from this specific site" from the downloading of them. That way the downloader can be generic and I can have downloader.Download(ebookUrls, downloadDirectoryName); where ebookUrls is just a list of links. – unknownprotocol Feb 27 '17 at 20:03

I have some issues with your validation.

public class BaseDownloader : IEbookDownloader
{
public Uri EbooksUrl { get; private set; }

The properties with private set are only set in the constructor, but PrepareToDownload() is the method that actually throws an exception if they're null. Even though the constructor can intentionally set them to null. That makes no sense. The constructor should set them to valid values, and if it cannot (due to problematic input), it should throw an exception right there and then, telling you what the problem is. Once that's done, there's no longer any need for validation in PrepareToDownload() because the object can only be constructed with valid state.
• Got it yea, I'll fix that. I meant to do some checking in PrepareToDownload() but in my haste to get the base class done I forgot that! – unknownprotocol Feb 27 '17 at 20:07