Using enumeration to report error messages legally is a good practice?

Question

I would like to combine my error messages and stuff into a single file and make my code more readable if possible.

Here is an example of what I have in the enum file:

public enum ZipErrorType { // START: define exception messages (alphabetical order) EMPTY_FILE_NAME_IN_LIST { public String toString() { return "One or more null/empty filename(s) found"; } }, FILE_DOESNT_EXIST { public String who(String sThisFile) { return "[" + sThisFile + "] does not exist"; } }, FILE_LIST_IS_NULL { public String toString() { return "File list is null/empty"; } }, FILENAME_NOT_ABSOLUTE { public String who(String sThisFile) { return "[" + sThisFile + "] is not absolute"; } }, MUST_BE_DIR { public String who(String sThisFile) { return "[" + sThisFile + "] must be a directory"; } }, MUST_BE_FILE { public String who(String sThisFile) { return "[" + sThisFile + "] must be a file"; } }, NULL_OR_EMPTY { public String who(String sThisFile) { return "[" + sThisFile + "] is null/empty"; } }, OUTPUT_FILE_ALREADY_EXISTS { public String who(String sThisFile) { return "[" + sThisFile + "] already exists"; } }, OUTPUT_FILENAME_EMPTY { public String toString() { return "Output filename is null/empty"; } }, OUTPUT_PATH_EMPTY { public String toString() { return "Output path is null/empty"; } }, // END: define exception messages NONE {}; public String who(String sThisFile) { return ""; } } 

Then in my program I have code like:

 private static ZipErrorType getFileErrorsIfAny(String sFilename, boolean shouldBeFile) { // check if given filename is absolute File file = new File(sFilename); if (!file.isAbsolute()) { return ZipErrorType.FILENAME_NOT_ABSOLUTE; } // check if file exists if (!file.exists()) { return ZipErrorType.FILE_DOESNT_EXIST; } // check if corresponding file is a file when it shouldn't be... if (file.isFile() && !shouldBeFile) { return ZipErrorType.MUST_BE_DIR; } // ...or a directory when it should be a file else if (file.isDirectory() && shouldBeFile) { return ZipErrorType.MUST_BE_FILE; } return ZipErrorType.NONE; } 

... and an example of using my enumeration:

  // check input files for (String sFile : files) { if (sFile == null || sFile.trim().length() == 0) { throw new NullPointerException("One or more filename is null/empty"); } errorIfAny = getFileErrorsIfAny(sFile.trim(), true); if (!errorIfAny.equals(ZipErrorType.NONE)) { throw new ZipInputException(errorIfAny.who(sFile.trim())); } } 

Now I know that it is difficult to judge only by these fragments of code, but is everything in order, from a general point of view? This is what I'm doing is not worth it, and is there a way to improve this?

Answer 1

I would suggest using simple string patterns instead of enumerations to generate error messages. Something like that:

 String EMPTY_FILE_NAME_IN_LIST_TEMPLATE = "One or more null/empty filename(s) found"; String FILE_DOESNT_EXIST_TEMPLATE = "[ %s ] does not exist"; String FILE_LIST_IS_NULL_TEMPLATE = "File list is null/empty"; String FILENAME_NOT_ABSOLUTE_TEMPLATE = "[ %s ] is not absolute"; String MUST_BE_DIR_TEMPLATE = "[ %s ] must be a directory"; String MUST_BE_FILE_TEMPLATE = "[ %s ] must be a file"; String NULL_OR_EMPTY_TEMPLATE = "[ %s ] is null/empty"; String OUTPUT_FILE_ALREADY_EXISTS_TEMPLATE = "[ %s ] already exists"; String OUTPUT_FILENAME_EMPTY_TEMPLATE = "Output filename is null/empty"; String OUTPUT_PATH_EMPTY_TEMPLATE = "Output path is null/empty"; 

And then use String.format(template, sFilename) to create the actual message.

You can also throw an exception from the getFileErrorsIfAny() method:

 File file = new File(sFilename); if (!file.isAbsolute()) { throw new ZipInputException(String.format(FILENAME_NOT_ABSOLUTE_TEMPLATE, sFilename)); } 

It looks cleaner and more compact for me.

Answer 2

It looks like this could lead to a lot of massive enum dotted around the code.

This is not the first time that someone has wanted to separate a journal message from a journal operator.

In fact, java.util.logging already has a structure for this that is intended for localization.

It uses a .properties file that contains messages.
You get a registrar indicating the file path in the class path: -

 Logger logger = Logger.getLogger("com.example", "path/to/messages.properties"); 

Registration applications are then executed using property keys

 logger.log(level, "messageKey"); 

And you can parameterize the log because it uses MessageFormat syntax

 zip.fileDoesNotExist={0} does not exist logger.log(level, "zip.fileDoesNotExist", file); 

These options are extremely flexible as you can specify formatting information in them and even use ChoiceFormat if necessary.

The main advantage of all this is that your messages are in a separate file, not a class . And you can enable and disable logging using the logging.properties file. You can even enable or disable logging for individual classes. And you can connect to several files to the console, you can send error messages, etc. etc.

So in conclusion. Use the existing logging structure. Do not roll.

Disclaimer: I am only talking about JUL, because then it is built into Java - you do not need any third-party libraries, there are many, many other frameworks.