views:

800

answers:

5

I have a number of icons used throughout an application - let's take ok/cancel icons as an example. At the moment they might be a tick and a cross (tick.png, cross.png) but I may want to replace them in future. Also, I would like to keep the resource path in one place.

Is this ok:

public class Icons {
    public static Icon OK = new ImageIcon(Icons.class.getResource("/icons/tick.png");
    public static Icon CANCEL = new ImageIcon(Icons.class.getResource("/icons/cross.png");
}

Or should I be doing this a different way? I don't mind relying on the existence of the image files at runtime since they're in the .jar

Solution

I've used Bent's idea for initialisation, and I've made the constants final:

public final class Icons {
    private static final Logger logger = Logger.getLogger(Icons.class);

    public static final Icon OK = icon("/icons/add.png");
    public static final Icon CANCEL = icon("/icons/cancel.png");

    private static Icon icon(String path) {
        URL resource = Icons.class.getResource(path);
        if(resource==null) {
            logger.error("Resource "+path+" does not exist");
            return new ImageIcon();
        }
        return new ImageIcon(resource);
    }
}
+2  A: 

You may want to mark the constants as final.

Markus
And the Icons class itself?
Draemon
A: 

That seems to be a fairly easy way to do that. Although I would name the images with the same name as what they are for ("ok.png", "cancel.png"). And make sure that it is clear that removing or renaming the images may cause issues.

Elie
+2  A: 

I see two problems with that, both might be acceptable:

  1. It will get hard to debug if your icons are not found or can't be loaded for some reasons. Code that runs in static initializers can be tricky, because it's easy to "loose" the exception.
  2. The class will probably never be unloaded and therefore the resource used by the Icons will never bee freed.

Number 1 is probably acceptable if you can make sure that the icons are always there and can even be worked around by putting the initialization in a static initializer block and adding good exception handling and logging.

Number 2 is probably acceptable since icons are usually used throughout the entire runtime of the application and they wouldn't be freed long before the application quits anyway.

So all in all I'd say that's fine.

Joachim Sauer
+1  A: 

If you want to keep you icons as static constants, I would extract the instantiation of the ImageIcon objects into a static method;

public static final Icon ok = icon("ok.png");


private static Icon icon(String path) {

    URL resource = Icons.class.getResource("/icons/" + path);
    if (resource == null) {
        // Log something...
        return null;
    }
    return new ImageIcon(resource);
}

This way you have control whenever something fail, and you don't have to repeat yourself in the instantiation.

Also, I would make the constants final.

A more general approach could be to use reflection to inspect your Icons-class and load resources for each public static Icon field in the class. This way, you would only have to declare a new Icon constant, and the corresponding resource would be loaded automatically based on the name of the constant. Leave a comment if you want more tips for doing this.

Bent André Solheim
I like this, although a) It should probably return Icon not ImageIcon, b) It might be better to return an empty icon rather than null.
Draemon
Reflection? Yuck. enum would be better. If we had "abstract enums" that would be useful.
Tom Hawtin - tackline
A: 

This seems to be the standard way of doing things, but I have had issues with before.

If you are using Eclipse with Maven and store these icons in maven's resource directory, when Eclipse does one of its automatic builds, it won't copy the icon files to your target/classes directory. This will cause a runtime exception when it can't find the icons.

You have to manually do a maven package at least once to get the icons in the right spot.

Pyrolistical
I don't user (or like) Eclipse or Maven, so this isn't a problem. This is a tool problem anyway, so I'd be inclined to fix the tool if I did run into something like this. Useful tip though.
Draemon