HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Avoid static initializer for lists of image and video formats?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
imageavoidvideoforformatslistsandinitializerstatic

Problem

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?

Solution

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).

Code Snippets

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[]{};
    }
}

Context

StackExchange Code Review Q#80084, answer score: 7

Revisions (0)

No revisions yet.