I'd like to consolidate my error messages and stuff into one file, and make my code more readable if possible.
Here's an example of what I have in my 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 how I make use of my enum:
// 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 it's hard to judge just by these code snippets alone, but is this alright, from a general perspective? Is what I'm doing not worth the trouble, and is there a way to improve this?
I would suggest using simple string templates instead of enums for building error messages. Something like this:
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)
for building actual message.
You may also consider throwing an exception right out of getFileErrorsIfAny()
method:
File file = new File(sFilename);
if (!file.isAbsolute()) {
throw new ZipInputException(String.format(FILENAME_NOT_ABSOLUTE_TEMPLATE, sFilename));
}
Looks cleaner and more compact to me.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With