Skip to main content
added 473 characters in body
Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391

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

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.

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

Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391

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.