patternjavaMinor
Avoid static initializer for lists of image and video formats?
Viewed 0 times
imageavoidvideoforformatslistsandinitializerstatic
Problem
I wrote the following code to read constants from annotations:
Is there a way to rewrite it better?
Or something else improve at this code?
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
then when that happens, the lines that set
Furthermore, it's good to minimize the scope of
Consider this reworked version:
Here, I don't need to ask what might throw the
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 (
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.