ctubbsii commented on code in PR #5478:
URL: https://github.com/apache/accumulo/pull/5478#discussion_r2047661240
##########
core/src/main/java/org/apache/accumulo/core/file/FilePrefix.java:
##########
@@ -18,25 +18,90 @@
*/
package org.apache.accumulo.core.file;
-import java.util.stream.Stream;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
public enum FilePrefix {
- BULK_IMPORT("I"), MINOR_COMPACTION("F"), MAJOR_COMPACTION("C"),
MAJOR_COMPACTION_ALL_FILES("A");
+ ALL("*"),
+ MINOR_COMPACTION("F"),
+ BULK_IMPORT("I"),
+ MAJOR_COMPACTION("C"),
+ MAJOR_COMPACTION_ALL_FILES("A"),
+ MERGING_MINOR_COMPACTION("M");
final String prefix;
FilePrefix(String prefix) {
this.prefix = prefix;
}
+ public String toPrefix() {
+ return this.prefix;
+ }
+
+ public String createFileName(String fileName) {
+ Objects.requireNonNull(fileName, "filename must be supplied");
+ Preconditions.checkArgument(!fileName.isBlank(), "Empty filename
supplied");
+ if (this == ALL || this == MERGING_MINOR_COMPACTION) {
+ throw new IllegalStateException(
+ "Unable to create filename with ALL, MERGING_MINOR_COMPACTION, or
UNKNOWN prefix");
+ }
+ return prefix + fileName;
+ }
+
public static FilePrefix fromPrefix(String prefix) {
- return Stream.of(FilePrefix.values()).filter(p ->
p.prefix.equals(prefix)).findAny()
- .orElseThrow(() -> new IllegalArgumentException("Unknown prefix type:
" + prefix));
+ Objects.requireNonNull(prefix, "prefix must be supplied");
+ Preconditions.checkArgument(!prefix.isBlank(), "Empty prefix supplied");
+ Preconditions.checkArgument(prefix.length() == 1, "Invalid prefix
supplied: " + prefix);
+ switch (prefix.toUpperCase()) {
+ case "A":
+ return MAJOR_COMPACTION_ALL_FILES;
+ case "C":
+ return MAJOR_COMPACTION;
+ case "F":
+ return MINOR_COMPACTION;
+ case "I":
+ return BULK_IMPORT;
+ case "M":
+ return MERGING_MINOR_COMPACTION;
+ default:
+ throw new IllegalArgumentException("Unknown prefix type: " + prefix);
Review Comment:
I don't mind changing this instance to a for loop, but I do find the 12 new
lines much more verbose and much less readable than the succinct 2 lines that
were there before, and don't see this as an overall improvement. I think it
would probably be more clearly an improvement if it didn't have the redundant
checks for the special cases being checked before the loop, since those are
already encapsulated by the check at the end of the loop. Those cases should be
rare, so there's not an optimization to bypass the loop for them. The verbosity
of the checks undermines any value the more specific error messages might have
offered. I think the succinct check at the end is sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
