Reddit RepostFinder Bot
public static bool DownloadImage(string url, bool isDirect) // returns if repost


C# supports something called XML Documentation Comments which allow you to document your code directly while writing it in a processor (as in IDE) friendly way.



public static bool DownloadImage(string url, bool isDirect) // returns if repost


Liar! This does not download the image but does checking on the URL, downloads the image, creates a MD5 sum for that image and then returns if it is already in the list of hashes.

You have one function that does everything, that's bad. Ideally you would split the responsibilities between multiple methods in one class, in some kind of such a structure (pseudo code):

class RepostChecker
    byte[] ComputeMD5(byte[] data)
    byte[] GetImage(string url)
    bool IsKnown(byte[] hash)


All of these methods can be static. So you can chain them like this:

IsKnown(ComputeMD5(GetImage(url)))


Or you can write a convenience method:

bool IsRepost(string url)


Which does the chaining for you. Then you define a class which does hold your hashes:

class Hashes
    static string HashToString(byte[] hash)

    void Add(string hash)
    bool Contains(string hash)
    void Load(whateverDatastoreYouUse)
    void Save(whateverDatastoreYouUse)


Internally this should use a HashSet&lt;String&gt;, not a normal List&lt;String&gt; like your implementation.

These changes would allow a more structured approach to the problem.



if (url.Contains('?'))
{
    url = url.Split('?')[0]; // if it has a query at the end like ?1
}
string filename = null;
if (url.Contains("imgur"))


You should use the Uri class if you work with URLs.

Uri uri = new Uri(url);           // http://imgur.com/blabla?whatever
uri.GetLeftPart(UriPartial.Path); // http://imgur.com/blabla
uri.Host;                         // imgur.com




Ideally you would not have one if {} else {} construct to handle this situation, but an interface which implementations and a static factory:

interface ImageHosterInteractor // Notice that the name is not optimal
    byte[] GetImage(string url)


From this interface you can now derive implementations for imgur and/or any other site. The static factory would look like this:

public static class ImageHosterInteractorFactory
{
    public static ImageHosterInteractor Create(Uri uri)
    {
        if (uri.PathAndQuery.EndsWith(".png")) // Or any other for that matter
        {
            // Implementation which simply loads the data from
            // the given Uri.
            return new ImageHosterInteractors.Plain();
        }

        // The uri does not end with an explicit ending,
        // so there's a good chance that we need to follow
        // redirections or perform some other magic.
        // So we use one of the explicit implementations.

        switch(uri.Host)
        {
            case "imgur.com":
                return new ImageHosterInteractors.Imgur();
                break;

            case "whatever.com":
                return new ImageHosterIntactors.Whatever();
                break;              

            default:
                throw new NotImplementedException("There is no implementation assigned to handle: " + uri.Host);

        }
    }
}


That allows your code to simply process the URL, pass it into the factory and get something back that knows how to treat that site and you don't need to worry about it further down.

Uri uri = new Uri(fromwhatever);
ImagerHosterInteractor interactor = ImageHosterInteractorFactory.Create(uri);
byte[] image = interactor.GetImage(uri);


Also adding new sites to handle is as easy as implementing ImageHosterInteractor and adding it to the factory method.



At the moment it looks like you're saving the images you download, is that necessary or is it enough to simply save the hashes?