Using Enum to apply Consumer

I have a utility called 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.)

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().getLastAccessTime());
entry.setLastModifiedTime(bean.entry().getLastModifiedTime());
try (InputStream in = bean.source().getInputStream(bean.entry())){
combiner.copyEntryFromSourceToTarget(in,entry,bean.zipOutputStream());
} catch (IOException e) {
throw new IllegalStateException(e);
}
});

BiConsumer<CombinerBean,ZipFileCombiner> biConsumer;

CollisionStrategy(BiConsumer<CombinerBean,ZipFileCombiner> consumer){
biConsumer = consumer;
}

void apply(CombinerBean bean, ZipFileCombiner combiner){
biConsumer.accept(bean,combiner);
}
}


CombinerBean

/*
CombinerBean is used in the CollisionStrategy.  It only serves to clean up code such that long arg lists
aren't necessary.
*/
private static class CombinerBean{
private final ZipEntry e;
private final ZipFile src;
private final ZipOutputStream out;

public CombinerBean(ZipOutputStream zo, ZipEntry ze, ZipFile source){
e = ze; src = source; out = zo;
}

public ZipEntry entry(){ return e; }
public ZipFile  source(){ return src; }
public ZipOutputStream zipOutputStream(){ return out; }
}


Method addEntryContent that refers to the strategy

private void addEntryContent(final ZipOutputStream out, final ZipFile source, final ZipEntry entry, final Set<String> entryNames) throws IOException {
// Assuming duplicate directory entries across archives are OK, so skip if directory
if(!entry.isDirectory()) {
LOGGER.warning(entry.getName() + " has already been added. Applying strategy: " + strategy);
strategy.apply(new CombinerBean(out, entry, source), this);
}
}else {
try (InputStream in = source.getInputStream(entry)) {
copyEntryFromSourceToTarget(in, entry, out);
}
}
}


Method copyEntryFromSourceToTarget which is called by the strategy

private void copyEntryFromSourceToTarget(final InputStream in, final ZipEntry targetEntry, final ZipOutputStream out) throws IOException {
out.putNextEntry(targetEntry);
out.closeEntry();
}

• Not an answer but I usually keep enum's in all capital letters or some other way to easily pick out. – Evan Carslake Jul 24 '15 at 15:45
• An alternative to wrapping a lambda is to make the enum CollisionStrategy implements BiConsumer<CombinerBean,ZipFileCombiner> – Peter Lawrey Jul 25 '15 at 5:52

• 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 dont do it in these methods. That's not consistent style.

• Thanks for the feedback. I agree with your points in general, but I don't apply blanket rules to my code. If a certain style for a block of code seems easier to read, even if it breaks conventions, then use it. I understand it's a subjective call, so maybe others don't feel the same. – MadConan Jul 27 '15 at 13:07
• @MadConan What are you referring to in detail? – GeroldBroser reinstates Monica Jul 27 '15 at 13:10
• CombinerBean, one statement per line. For the simplistic, one-line-code-blocks in a small inner class, I personally prefer seeing it all together at a glance as opposed to scrolling back and forth. – MadConan Jul 27 '15 at 13:13
• The String.format` suggestion is a good one, though. I need to start using that more. Old habits die hard :) – MadConan Jul 27 '15 at 13:16
• @MadConan With 8 more lines you have to scroll? Are you developing on a smartphone in landscape view? :-) – GeroldBroser reinstates Monica Jul 27 '15 at 13:19