Change desktop background

Question

I've created a simple program that downloads an image (different image every day), then sets it as my desktop background.

It is a console application, and closes immediately after downloading the image and setting it as my desktop background. It runs every morning at 6:00 AM via Task Scheduler.

Although I have significantly improved my program structure, I want to know if there are any poor practices currently in my code, or improvements that can be made.

Link to the original program:

The updated, final version is formatted below.

using Microsoft.Win32;
using Newtonsoft.Json;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.IO;
using System.Linq;
using System.Net;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Background
{
    class Program
    {
        static void Main(String[] args)
        {
            String URL = getBackgroundURL();
            Image background = downloadBackground(URL + getResolutionExtension());
            saveBackground(background);
            setBackground(background, PicturePosition.Fill);
        }

        public static String getBackgroundURL()
        {
            using(WebClient webClient = new WebClient()) 
            {
                    Console.WriteLine("Downloading JSON...");
                    String jsonString = webClient.DownloadString("super secret URL");
                    dynamic jsonObject = JsonConvert.DeserializeObject<dynamic>(jsonString);
                    String backgroundURL = "image_url.com" + jsonObject.images[0].urlbase;
                    Console.WriteLine("Downloaded JSON!\n");    
                    return backgroundURL;
            }
        }

        public static Boolean websiteExists(String URL)
        {
            try
            {
                HttpWebRequest request = WebRequest.Create(URL) as HttpWebRequest;
                request.Method = "HEAD";
                HttpWebResponse response = request.GetResponse() as HttpWebResponse;
                return(response.StatusCode == HttpStatusCode.OK);
            }
            catch {return false;}
        }

        public static String getResolutionExtension()
        {
            Rectangle resolution = Screen.PrimaryScreen.Bounds;
            String potentialURL = "_" + resolution.Width + "x" + resolution.Height + ".jpg";
            if(websiteExists(potentialURL)) return potentialURL;
            else return "_1920x1080.jpg";
        }

        public static Image downloadBackground(String URL)
        {
                Console.WriteLine("Downloading background...");
                HttpWebRequest httpWebRequest =(HttpWebRequest)HttpWebRequest.Create(URL);
                HttpWebResponse httpWebReponse =(HttpWebResponse)httpWebRequest.GetResponse();
                Stream stream = httpWebReponse.GetResponseStream();
                Image background = Image.FromStream(stream);
                Console.WriteLine("Downloaded background!\n");
                return background;
        }

        public static String getBackgroundPath()
        {
            String directory = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures) + "/Backgrounds/";
            Directory.CreateDirectory(directory);
            return Path.Combine(directory, DateTime.Now.ToString("M-d-yyyy")+".bmp");
        }

        public static Boolean saveBackground(Image background)
        {
            try
            {
                Console.WriteLine("Saving background...");
                background.Save(getBackgroundPath(), System.Drawing.Imaging.ImageFormat.Bmp);
                Console.WriteLine("Saved background!\n");
                return true;
            }
            catch {return false;}
        }

        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...");
            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;
            }
            key.Close();

            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, getBackgroundPath(), UPDATE_INI_FILE | SEND_WINDOWS_INI_CHANGE);
            Console.WriteLine("Set background!\n");
        }

    }
}

Answer 1

These using directives are not needed and can be safely removed:

using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

Method names should be PascalCase:

• getBackgroundURL => GetImageUrl
• websiteExists => WebsiteExists
• getResolutionExtension => GetResolutionExtension
• downloadBackground => DownloadImage
• getBackgroundPath => GetImageFilePath
• saveBackground => SaveBackground - this would be more precise as SaveImageFile
• setBackground => SetBackground - this would be more precise as SetDesktopWallpaper

URL as part of a member name, should be Url; as a parameter name, should be url.

I would move the nested types PicturePosition and NativeMethods to another file, named after the type (respectively, PicturePosition.cs and NativeMethods.cs).

In this snippet:

RegistryKey key = Registry.CurrentUser.OpenSubKey(@"Control Panel\Desktop", true);
switch (style)
{
    case PicturePosition.Tile:
        key.SetValue(@"PicturePosition", "0");
        key.SetValue(@"TileWallpaper", "1");
        break;

There's a possibility for a NullReferenceException on every access of the key object, including key.Close() - also, since .net 4.0 the RegistryKey implements IDisposable, which means it should be wrapped with a using block and not closed explicitly:

try
{
    using(var key = Registry.CurrentUser.OpenSubKey(@"Control Panel\Desktop", true))
    {
        switch (style)
        {
            //...
        }
    }
}
catch (NullReferenceException)
{
    Console.WriteLine("Specified registry key was not found.");
}

I'm not a big fan of that switch block, but abstracting a concept here would probably be overkill.

The verbatim specifier on @"PicturePosition" and @"TileWallpaper" isn't needed, as there are no escapes/backslashes in these strings.

I like that your type declarations are consistently explicit, but I would personally prefer them consistently implicit (i.e. using var) - but the keyword here is only "consistently", so that's just my own personal preference here.

This notation makes it hard to, say, add attributes to your enums, or just to maintain them in general:

public enum PicturePosition
{
    Tile, Center, Stretch, Fit, Fill
}

Enum members should be laid out vertically:

public enum PicturePosition
{
    Tile,
    Center,
    Stretch,
    Fit,
    Fill
}

---

This is a nice missed opportunity for string.Concat:

String potentialURL = "_" + resolution.Width + "x" + resolution.Height + ".jpg";

Overall I like your methods, they're small and relatively specialized, and are generally well named. I'd consider regrouping them into similar-themed classes:

• WebClient and JsonConvert-related stuff into one.
• IO-related stuff into another.

This would only leave Main and SetBackground in the Program class, resulting in 3 smaller, specialized classes.

Answer 2

Image also implements IDisposable, therefore wrap in using:

using (Image background = downloadBackground(URL + getResolutionExtension()))
{
    saveBackground(background);
    setBackground(background, PicturePosition.Fill);
}

Ok, more info: Image may utilize a sizable amount of memory, depending on the image, so executing deterministic disposal of those resources is incumbent upon the developer in order to ensure best application behavior. Read here for additional information.

Answer 3

In addition to what @Mathieu Guidon wrote, to make re-use (copying code to other projects) easier, instead of referencing

using Microsoft.Win32;
using Newtonsoft.Json;

I would fully qualify calls to those, i.e.

RegistryKey key = Microsoft.Win32.Registry.CurrentUser.OpenSubKey(@"Control Panel\Desktop", true);

dynamic jsonObject = Newtonsoft.Json.JsonConvert.DeserializeObject(jsonString);

and all other occurances like that (if they are candidates for re-use, of course). After you have done that, right click in the editor and select "Remove and sort usings" (or use the chord Ctrl+R, Ctrl+G). This will keep only the remaining required usings.

And I would take advantage of XML comments (just type above your method /// and press enter) to self-document the methods you're writing, i.e.:

/// <summary>
/// Sets background image of current user's desktop
/// </summary>
/// <param name="backgroundFilePath">Path to picture file (*.bmp or *.jpg)</param>
/// <param name="style">One of enum PicturePosition</param>
/// <remarks>optionally put some remarks here</remarks>
public static void SetBackground(string backgroundFilePath, PicturePosition style)
{
    Console.WriteLine("Setting background...");
    // ... code as you provided ..
}

This allows you to fully take advantage of Microsoft's Intellisense, i.e. just point on the name of the method and get its description and parameters (more about it is available here).

And, only a small thing, I changed the method name to uppercase and renamed the first parameter to backgroundFilePath to make it more clear it is a file and not a directory.