Get icon from website, show and update icon

Question

I was thinking the other day how could I get the icon from a website and make it show and update when it is updated on the site so I made this code:

URL url2 = null;     
boolean updateIcon = true;
try {
    url2 = new URL("https://dl.dropboxusercontent.com/s/70rhm66myw8ay2v/Perception%20Logo.png");
} catch (MalformedURLException ex) {
    updateIcon = false;
    ex.printStackTrace();
}
if (updateIcon) {
    System.out.println("[Perception] Icon updated.");
    if (ComputerCheck.isWindows()) {
        Image bimg = Toolkit.getDefaultToolkit().getImage(url2);
        this.setIconImage(bimg);
    } else if (ComputerCheck.isMac()) {
        Application application = Application.getApplication();
        Image image = Toolkit.getDefaultToolkit().getImage(url2);
        application.setDockIconImage(image);  
    }
}

And it works 100% fine but it makes the startup time a lot slower. I was wondering how could I optimize this.

Answer 1

I'd restructure this code significantly. The first thing I'd do is move the image-retrieval into a separate method and the image placing would then be "simpler"..

Compare your code against:

Image image = getRemoteLogo();
if (image != null) {
    // Logo updated
    updateLogo(image);
}

This is largely reimagining what you're doing, and depending on how you prefer it, you might move the nullcheck into updateLogo.

This allows you to hide the image and get rid of the IMO .. clunky null-initialization before the try-block:

private Image getRemoteLogo() {
    try {
       URL remoteUrl = new URL(Config.getRemoteLogoUrl());
       return Toolkit.getDefaultToolkit().getImage(remoteUrl);
    } catch (MalformedURLException ex) {
       // this is the point where you can slap the programmer :D
       ex.printStackTrace();
       return null;
    }
}

This neatly hides the hardcoded dropbox URL in a (not further specified) Configuration, which allows changing the logo location without having to replace classes.

Also it cleanly abstracts away the Image-loading and has clear behavior in error cases. It also gets rid of the superfluous boolean flag you have.

The updateLogo(Image) is history then, you do your ComputerCheck and set the image accordingly.

The startup slowdown is something you'll have to live with.

Oh and I changed the variable names slightly. Use speaking names wherever possible, future maintainers (including yourself) may not be able to map out which number corresponds to which URL :)

Answer 2

I'd personally go one step further than the previous answer. I hate null-checking and therefore passing null as arguments and/or as return value. The only reason for me to do that would be if it was in an environment where performance is very important. This does not appear to be the case here. So here's my take:

public void tryToUpdateLogo(URL remoteUrl)
{
    try{
        updateLogo(remoteURL);
    }
    catch(MalformedURLException, UnknownSystemException ex) {
        ex.printStackTrace();
        // HANDLE ERROR
    }
}
private void updateLogo() throws MalformedURLException
{
    Image logo = getLogoFromURL(URL remoteUrl);
    UpdateLogo(logo);
}
private Image getLogoFromURL(URL remoteURL)
{
    return Toolkit.getDefaultToolkit().getImage(remoteUrl);
}
private void updateLogo(Image logo) throws UnknownSystemException
{
    System.out.println("[Perception] Icon updated.");

    if(ComputerCheck.isWindows())
        this.setIconImage(logo);
    else if(ComputerCheck.isMac())
        getMACApplication().setDockIconImage(logo);
    else
        throw new UnknownSystemException();
}

This code is based on the premise that "Error handling is a responsability", which due to the SRP (Single Responsability Principle) is supposed to be separated from the code as is described in Robert C. Martins Clean Code .