Avoid static initializer for lists of image and video formats?

Question

I wrote the following code to read constants from annotations:

public class ContentUtils {
    static {
        String[] videoFormats = null;
        String[] imageFormats = null;
        try {
            Field multipartFileField = MultipartFileWrapper.class.getField("multipartFile");
            Extensions annotation = multipartFileField.getAnnotation(Extensions.class);
            imageFormats = annotation.imageFormats();
            videoFormats = annotation.videoFormats();
        } catch (NoSuchFieldException e) {
            if (imageFormats == null) {
                imageFormats = new String[]{};
            }
            if (videoFormats == null) {
                videoFormats = new String[]{};
            }
        }
        ACCEPTED_IMAGE_FORMATS = imageFormats;
        ACCEPTED_VIDEO_FORMATS = videoFormats;
    }

    public static final String[] ACCEPTED_IMAGE_FORMATS;
    public static final String[] ACCEPTED_VIDEO_FORMATS;
}

Is there a way to rewrite it better?

Or something else improve at this code?

Answer 1

According to your comment, if it's only the MultipartFileWrapper.class.getField("multipartFile") statement that might throw a NoSuchFieldException, then when that happens, the lines that set imageFormats and videoFormats will not be reached, so these values will remain null, and the if conditions in the catch block are pointless.

Furthermore, it's good to minimize the scope of try-catch blocks. Consider this reworked version:

static {
    Field multipartFileField = null;
    try {
        multipartFileField = MultipartFileWrapper.class.getField("multipartFile");
    } catch (NoSuchFieldException e) {
        // maybe log that something bad happened?
    }
    if (multipartFileField != null) {
        Extensions annotation = multipartFileField.getAnnotation(Extensions.class);
        ACCEPTED_IMAGE_FORMATS = annotation.imageFormats();
        ACCEPTED_VIDEO_FORMATS = annotation.videoFormats();
    } else {
        ACCEPTED_IMAGE_FORMATS = new String[]{};
        ACCEPTED_VIDEO_FORMATS = new String[]{};
    }
}

Here, I don't need to ask what might throw the NoSuchFieldException, it's obvious, which is good.

As for avoiding the static initializer... if you want to have static fields with non-trivial constants, then there's just no other way, you have no choice but to initialize them in a static initialization block like this. Other option would be to give up the static fields, make them non-static and initialize in the constructor. But this probably won't make much sense, since the field values come from static data (MultipartFileWrapper.class).

Answer 2

The worst part is actually

public static final String[] ACCEPTED_IMAGE_FORMATS;
public static final String[] ACCEPTED_VIDEO_FORMATS;

publishing both arrays for everyone to modify them at will. Ideally, use Guava's ImmutableList. Otherwise, use Collections.unmodifiableList(new ArrayList(....)).

---

If you don't mind letting the initialization do some trivial work twice, you can write

private static ImmutableList<String> initAcceptedFormats(boolean forImage) {
    Field multipartFileField;
    try {
        multipartFileField = MultipartFileWrapper.class.getField("multipartFile");
    } catch (NoSuchFieldException e) {
        return ImmutableList.<String>of();
    }
    Extensions annotation = multipartFileField.getAnnotation(Extensions.class);
    return ImmutableList.copyOf(forImage ? annotation.imageFormats() : annotation.videoFormats());
}

public static final ImmutableList<String> ACCEPTED_IMAGE_FORMATS = initAcceptedFormats(true);
public static final ImmutableList<String> ACCEPTED_VIDEO_FORMATS = initAcceptedFormats(false);

---

I'd go for it. The use of the boolean forImage is not nice, but for a private two-shots method it's better than introducing an enum or whatever. I'd also avoid the daniel' Java 8 idea as (while it's nice), it adds a new interface and two hidden anonymous classes.

---

In my solution, annotation gets computed twice. This "problem" should be surely ignored, but in case of a repeated expensive computation, a private static final variable can be introduced.

Answer 3

You can use a private static Method to initialize the fields.

public class ContentUtils {
  public static final String[] ACCEPTED_IMAGE_FORMATS = getAcceptedImageFormats();
  public static final String[] ACCEPTED_VIDEO_FORMATS = getAcceptedVideoFormats();

  private static String[] getAcceptedImageFormats() {
        try {
            Field multipartFileField = MultipartFileWrapper.class.getField("multipartFile");
            Extensions annotation = multipartFileField.getAnnotation(Extensions.class);
            return annotation.imageFormats();
        } catch (NoSuchFieldException e) {
            return new String[0];
        }
    }
    private static String[] getAcceptedVideoFormats() {
      // ...
    }
}

Is it better? I don't know. The Oracle Documentation says you can use this method later to reinitialize the variables.

The duplication that results, with this almost identical Methods can be eliminated with lambda Expressions, if Java 8 is available.

public class ContentUtils {
  public static final String[] ACCEPTED_IMAGE_FORMATS = getAcceptedFormats(a -> a.imageFormats());
  public static final String[] ACCEPTED_VIDEO_FORMATS = getAcceptedFormats(a -> a.videoFormats());

  private static String[] getAcceptedFormats(FormatAcceptor acc) {
        try {
            Field multipartFileField = MultipartFileWrapper.class.getField("multipartFile");
            Extensions annotation = multipartFileField.getAnnotation(Extensions.class);
            return acc.acceptedFormats(annotation);
        } catch (NoSuchFieldException e) {
            return new String[0];
        }
    }

  interface FormatAcceptor {
    String[] acceptedFormats(Extensions annotation);
  }
}