patternjavaMinor
Using Enum to apply Consumer
Viewed 0 times
enumconsumerusingapply
Problem
I have a utility called
Full code here in GitHub. (
Relevant pieces:
```
public enum CollisionStrategy {
FAIL((bean, combiner) -> {
throw new IllegalStateException("Collision detected. Entry " + bean.entry() +
" exists. Current source: " + bean.source());
}),
USE_FIRST((bean, combiner) -> {}),
RENAME_AND_ADD((bean, combiner) -> {
if(bean.entry().isDirectory()){
return;
}
String[] sourceNameParts = bean.source().getName().split("/");
String sourceName = sourceNameParts[sourceNameParts.length-1];
String[] entryNameParts = bean.entry().getName().split("/");
String entryName = entryNameParts[entryNameParts.length-1];
entryNameParts[entryNameParts.length-1] = sourceName + entryName;
String newEntryName = Arrays.asList(entryNameParts).stream().collect(Collectors.joining("/"));
// Have to manually "clone" the entry. This sucks.
ZipEntry entry = new ZipEntry(newEntryName);
entry.setTime(bean.entry().getTime());
entry.setComment(bean.entry().getComment());
entry.setCompressedSize(bean.entry().getCompressedSize());
entry.setCrc(bean.entry().getCrc());
entry.setCreationTime(bean.entry().getCreationTime());
entry.setMethod(bean.entry().getMethod());
entry.setExtra(bean.entry().getExtra());
entry.setLastAccessTime(bean.entry().g
ZipFileCombiner that combines zip archives into a single archive. Part of the implementation is using an Enum called CollisionStrategy to deal with entry collisions. It's a nested Enum. It seems kind of ugly to me. It feels like embedding that much code within the Enum seems wrong. I could create yet another inner class (or outer) to hold it, but that seems wrong as well since logic is tightly coupled to the idea of a strategy. I'm looking for suggestions as well as a general peer review.Full code here in GitHub. (
IOUtil is also in that project.)Relevant pieces:
CollisionStrategy```
public enum CollisionStrategy {
FAIL((bean, combiner) -> {
throw new IllegalStateException("Collision detected. Entry " + bean.entry() +
" exists. Current source: " + bean.source());
}),
USE_FIRST((bean, combiner) -> {}),
RENAME_AND_ADD((bean, combiner) -> {
if(bean.entry().isDirectory()){
return;
}
String[] sourceNameParts = bean.source().getName().split("/");
String sourceName = sourceNameParts[sourceNameParts.length-1];
String[] entryNameParts = bean.entry().getName().split("/");
String entryName = entryNameParts[entryNameParts.length-1];
entryNameParts[entryNameParts.length-1] = sourceName + entryName;
String newEntryName = Arrays.asList(entryNameParts).stream().collect(Collectors.joining("/"));
// Have to manually "clone" the entry. This sucks.
ZipEntry entry = new ZipEntry(newEntryName);
entry.setTime(bean.entry().getTime());
entry.setComment(bean.entry().getComment());
entry.setCompressedSize(bean.entry().getCompressedSize());
entry.setCrc(bean.entry().getCrc());
entry.setCreationTime(bean.entry().getCreationTime());
entry.setMethod(bean.entry().getMethod());
entry.setExtra(bean.entry().getExtra());
entry.setLastAccessTime(bean.entry().g
Solution
-
It's best practices to let statements like
I'd use:
instead of string concatenation:
The same applies to
Java best practices is "one statement per line" since your code is a lot more read (possibly by others, too) than it is written.
You break lines at ~104 in
It's best practices to let statements like
if, for, ... be followed by a space to disinguish them from method invocations. - CollisionStrategy
I'd use:
String.format("Collision detected. Entry %s exists. Current source: %s",
bean.entry(), bean.source());instead of string concatenation:
"Collision detected. Entry " + bean.entry() +
" exists. Current source: " + bean.source()The same applies to
addEntryContent().- CombinerBean
Java best practices is "one statement per line" since your code is a lot more read (possibly by others, too) than it is written.
- addEntryContent() and copyEntryFromSourceToTarget()
You break lines at ~104 in
CollisionStrategy but you don`t do it in these methods. That's not consistent style.Code Snippets
String.format("Collision detected. Entry %s exists. Current source: %s",
bean.entry(), bean.source());"Collision detected. Entry " + bean.entry() +
" exists. Current source: " + bean.source()Context
StackExchange Code Review Q#97955, answer score: 2
Revisions (0)
No revisions yet.