4
\$\begingroup\$

I've created a simple command-line program that downloads the Bing Image of the Day, then sets it as my desktop background.

It is a console application, and closes immediately after setting my desktop background. It runs daily via Task Scheduler.

I've been working on this as a mini project for a while and would like to know if I am employing any poor coding practices or if there are ways to improve efficiency. The project can be found on GitHub. The executable is located at BingBackground/BingBackground/BingBackground/bin/Release/BingBackground.exe

static void Main(String[] args)
{
    String urlBase = GetBackgroundURLBase();
    Image background = DownloadBackground(urlBase + GetResolutionExtension(urlBase));
    SaveBackground(background);
    SetBackground(background, PicturePosition.Fill);
}
public static String GetBackgroundURLBase()
{
    using (WebClient webClient = new WebClient())
    {
        Console.WriteLine("Downloading JSON...");
        String jsonString = webClient.DownloadString("https://www.bing.com/HPImageArchive.aspx?format=js&idx=0&n=1&mkt=en-US");
        dynamic jsonObject = JsonConvert.DeserializeObject<dynamic>(jsonString);
        return "https://www.bing.com" + jsonObject.images[0].urlbase;
    }
}
public static String GetBackgroundTitle()
{
    using (WebClient webClient = new WebClient())
    {
        String jsonString = webClient.DownloadString("https://www.bing.com/HPImageArchive.aspx?format=js&idx=0&n=1&mkt=en-US");
        dynamic jsonObject = JsonConvert.DeserializeObject<dynamic>(jsonString);
        String copyrightText = jsonObject.images[0].copyright;
        return copyrightText.Substring(0, copyrightText.IndexOf(" ("));
    }
}
public static Boolean WebsiteExists(String url)
{
    try
    {
        WebRequest request = WebRequest.Create(url);
        request.Method = "HEAD";
        HttpWebResponse response = (HttpWebResponse)request.GetResponse();
        return (response.StatusCode == HttpStatusCode.OK);
    }
    catch
    {
        return false;
    }
}
public static String GetResolutionExtension(String url)
{
    Rectangle resolution = Screen.PrimaryScreen.Bounds;
    String potentialExtension = "_" + resolution.Width + "x" + resolution.Height + ".jpg";
    if (WebsiteExists(url + potentialExtension))
    {
        Console.WriteLine("Background for " + resolution.Width + "x" + resolution.Height + " found.");
        return potentialExtension;
    }
    else
    {
        Console.WriteLine("No background for " + resolution.Width + "x" + resolution.Height + " was found.");
        Console.WriteLine("Using 1920x1080 instead.");
        return "_1920x1080.jpg";
    }
}
public static Image DownloadBackground(String url)
{
    Console.WriteLine("Downloading background...");
    WebRequest request = WebRequest.Create(url);
    WebResponse reponse = request.GetResponse();
    Stream stream = reponse.GetResponseStream();
    return Image.FromStream(stream);
}
public static String GetBackgroundImagePath()
{
    String directory = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures) + "/Bing Backgrounds/" + DateTime.Now.Year.ToString();
    Directory.CreateDirectory(directory);
    return Path.Combine(directory, DateTime.Now.ToString("M-d-yyyy") + ".bmp");
}
public static void SaveBackground(Image background)
{
    Console.WriteLine("Saving background...");
    background.Save(GetBackgroundImagePath(), System.Drawing.Imaging.ImageFormat.Bmp);
}
public enum PicturePosition
{
    Tile,
    Center,
    Stretch,
    Fit,
    Fill
}
internal sealed class NativeMethods
{
    [DllImport("user32.dll", CharSet = CharSet.Auto)]
    internal static extern int SystemParametersInfo(int uAction, int uParam, String lpvParam, int fuWinIni);
}
public static void SetBackground(Image background, PicturePosition style)
{
    Console.WriteLine("Setting background...");
    using (RegistryKey key = Registry.CurrentUser.OpenSubKey(@"Control Panel\Desktop", true))
    {
        switch (style)
        {
            case PicturePosition.Tile:
                key.SetValue("PicturePosition", "0");
                key.SetValue("TileWallpaper", "1");
                break;
            case PicturePosition.Center:
                key.SetValue("PicturePosition", "0");
                key.SetValue("TileWallpaper", "0");
                break;
            case PicturePosition.Stretch:
                key.SetValue("PicturePosition", "2");
                key.SetValue("TileWallpaper", "0");
                break;
            case PicturePosition.Fit:
                key.SetValue("PicturePosition", "6");
                key.SetValue("TileWallpaper", "0");
                break;
            case PicturePosition.Fill:
                key.SetValue("PicturePosition", "10");
                key.SetValue("TileWallpaper", "0");
                break;
        }
    }
    const int SET_DESKTOP_BACKGROUND = 20;
    const int UPDATE_INI_FILE = 1;
    const int SEND_WINDOWS_INI_CHANGE = 2;
    NativeMethods.SystemParametersInfo(SET_DESKTOP_BACKGROUND, 0, GetBackgroundImagePath(), UPDATE_INI_FILE | SEND_WINDOWS_INI_CHANGE);
}
\$\endgroup\$
4
\$\begingroup\$

Great idea, and very nicely written code, a pleasure to read even for a non-C# guy: decomposed to many small methods with single responsibility, using using, well-indented, well-spaced (almost, see below).

Don't fetch the same URL twice

GetBackgroundURLBase and GetBackgroundTitle fetch the same URL. This is unnecessary network bandwidth, duplicated logic, and duplicated magic strings. It would be better to have a method to get the jsonObject, and then pass that to a method that extracts the base URL, and another that extracts the title.

Avoid magic strings

There are many magic strings scattered around in the code that would be better as constants, defined at the top of the class where they are easy to see:

  • https://www.bing.com/HPImageArchive.aspx?format=js&idx=0&n=1&mkt=en-US
  • _1920x1080.jpg
  • /Bing Backgrounds/
  • M-d-yyyy

Avoid hard coding

It would be nice to control some elements with configuration instead of hard coding, for example: the destination directory, the date format, and possibly the image service URL too in case it might change.

Logging

The program is quite chatty, writing to console many of the things it is doing. But one important piece of detail is missed in WebsiteExists: the method will return false some very different scenarios:

  1. 404 error, page not found -- the main intended purpose
  2. Other non-200 HTTP status code; quietly ignored
  3. Exception thrown; quietly ignored

It might be good to distinguish these different cases, so that the program reports what happens more accurately than just "resolution AxB was not found".

Related to this, when writing error messages, probably Console.Error.WriteLine is better than Console.WriteLine.

String formatting

This maybe a matter of taste, but instead of this:

Environment.GetFolderPath(Environment.SpecialFolder.MyPictures) + 
    "/Bing Backgrounds/" + DateTime.Now.Year.ToString();

I prefer using String.Format, to make the final string structure easier to see thanks to the template, and to get rid of ToString() calls, like this:

String.Format("{0}/Bing Backgrounds/{1}", 
    Environment.GetFolderPath(Environment.SpecialFolder.MyPictures), DateTime.Now.Year);

Update

Actually, as @mjolka pointed out, when constructing a path, String.Format is not a good solution either, because the right way to do that is using Path.Combine, as you already did that correctly in other places of the code.

Almost well-spaced

The horizontal spacing is spot on, with spaces around operators. The indentation is also perfect. The one thing missing is empty lines between methods, for improved readability by better visual separation.

\$\endgroup\$
  • \$\begingroup\$ Nice answer, only thing I would suggest is using Path.Combine instead of string.Format. \$\endgroup\$ – mjolka Jun 7 '15 at 8:24
  • \$\begingroup\$ Good call! Updated. \$\endgroup\$ – Stop ongoing harm to Monica Jun 7 '15 at 9:00
  • \$\begingroup\$ All excellent points. As far as the spacing, I left out extra spacing/comments simply to minify the amount of code readers needed to look at. Thanks for your insight! \$\endgroup\$ – Josue Espinosa Jun 7 '15 at 21:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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