patternjavaMinor
Generating classes from enum
Viewed 0 times
fromenumgeneratingclasses
Problem
I have a system that receives messages. Each message has a type defined. These types are declared in an enum:
People can extend my system by implementing message processors. Each message type can only have one processor and most will have none. My current design has users writing their processors and I have one factory that they have to change:
My problem with this is twofold:
I thought of maybe solving this using reflection:
This solution solves the two issues I mentioned above.
My questions are:
enum MessageType {
TYPE1, TYPE2, ... , TYPE999
}People can extend my system by implementing message processors. Each message type can only have one processor and most will have none. My current design has users writing their processors and I have one factory that they have to change:
public static Map getProcessors() {
Map processors = Maps.newHashMap();
processors.put(TYPE33, new Type33Processor());
processors.put(TYPE111, new Type111Processor());
processors.put(TYPE222, new Type222Processor());
processors.put(TYPE444, new Type444Processor());
return processors;
}My problem with this is twofold:
- Each new processor has to remember to add this boilerplate.
- This factory has to know all processor types.
I thought of maybe solving this using reflection:
public static Map getProcessors() {
Map processors = Maps.newHashMap();
Reflections reflections = new Reflections(getClass().getPackage().getName());
Set> allProcessors =
reflections.getSubTypesOf(MessageProcessor.class);
for (Class processorClass : allProcessors) {
MessageProcessor processor = processorClass.getConstructor().newInstance();
processors.put(processor.getMessageType(), processor);
}
return processors;
}This solution solves the two issues I mentioned above.
My questions are:
- I understand that reflection has dangers and pitfalls. Is this a valid scenario to use it?
- Is there a better way to improve my first solution without having to resort to using reflection?
Solution
I understand your concerns here. This is a messy situation, and Java (like most languages, I believe), makes it relatively hard to build up interdependencies during the bootstrapping of the classes, classloaders, etc.
What would be ideal is if you could have an Enum that allowed the following:
but, that would require the MessageType to be constructed knowing what it's processor is.... but you need to map that on a case-by-case basis, based on the factory that supplies them.
What is certainly not ideal, is having public static methods that create new Maps each time they are called....
Why does it need to be public... what happens if it is called multiple times?
That's an ugly solution.
The ideal solution would be to construct the MessageProcessor at the same time as the MessageType, but, since that can't be done (as the MessageType is a parameter to the factory), I would instead recommend that you create a synchronized, or locked, or atomic method that is part of the Enum, and handles the situation for you. If your factory has the method:
then your enum could have a private setup like;
This is a compromize I use quite often. It gives you an abstraction between the class-load initialization, and the runtime usage of the enum.
It has the following benefits:
on the down-side, you have the following:
What would be ideal is if you could have an Enum that allowed the following:
MessageProcessor processor = message.getMessageType().getProcessor();but, that would require the MessageType to be constructed knowing what it's processor is.... but you need to map that on a case-by-case basis, based on the factory that supplies them.
What is certainly not ideal, is having public static methods that create new Maps each time they are called....
public static Map getProcessors() { ...Why does it need to be public... what happens if it is called multiple times?
That's an ugly solution.
The ideal solution would be to construct the MessageProcessor at the same time as the MessageType, but, since that can't be done (as the MessageType is a parameter to the factory), I would instead recommend that you create a synchronized, or locked, or atomic method that is part of the Enum, and handles the situation for you. If your factory has the method:
public MessageProcessor getMessageProcessorFor(MessageType mType) {
.....
}then your enum could have a private setup like;
enum MessageType {
TYPE1, TYPE2, ... , TYPE999;
private final AtomicReference processor = new AtomicReference<>();
public MessageProcessor getProcessor() {
MessageProcessor proc = processor.get();
if (proc != null) {
return proc;
}
proc = FactoryClass.getmessageProcessorFor(this);
if (processor.compareAndSet(null, proc)) {
return proc;
}
// there was a race condition, we lost, so use the winner
return processor.get();
}
}This is a compromize I use quite often. It gives you an abstraction between the class-load initialization, and the runtime usage of the enum.
It has the following benefits:
- you have a simpler run-time execution model.
- your exceptions, if any, related to creating the MessageProcessor have better traces.
- you do not have complicated circular dependencies at construct time.
- you only create instances for types you use.
on the down-side, you have the following:
- slight performance hit each time you call the method as it checks its state.
- well, no 2, really.
Code Snippets
MessageProcessor processor = message.getMessageType().getProcessor();public static Map<MessageType, MessageProcessor> getProcessors() { ...public MessageProcessor getMessageProcessorFor(MessageType mType) {
.....
}enum MessageType {
TYPE1, TYPE2, ... , TYPE999;
private final AtomicReference<MessageProcessor> processor = new AtomicReference<>();
public MessageProcessor getProcessor() {
MessageProcessor proc = processor.get();
if (proc != null) {
return proc;
}
proc = FactoryClass.getmessageProcessorFor(this);
if (processor.compareAndSet(null, proc)) {
return proc;
}
// there was a race condition, we lost, so use the winner
return processor.get();
}
}Context
StackExchange Code Review Q#91160, answer score: 4
Revisions (0)
No revisions yet.