I’ve got a little problem in choosing the best design.
I have some (5 at the moment) image processing operations in my coe (Java). Every processing step independent from the other ones and consoists of a small amount of code, 5-15 lines in general. These are mostly wrappers to JAI, AWT and so on.
My current design is an abstract class:
public abstract class AbstractPreProcessingStep {
protected Logger logger = Logger.getLogger(this.getClass().getName());
protected AbstractPreProcessingStep() {
};
public abstract BufferedImage startProcessing(BufferedImage input);
}
Every processing step implements this class and can be called uniform. (like the Strategy-Pattern).
Since every processing step may have a different set of parameters, I created a factory class, like this:
public class PreProcessingFactory {
public static AbstractPreProcessingStep createColorInverter() {
return new ColorInverter();
}
public static AbstractPreProcessingStep createRescaler(float rescaleFactor) {
return new Rescaler(rescaleFactor);
}
public static AbstractPreProcessingStep createEdgeDetector() {
return new EdgeDetector();
}
public static AbstractPreProcessingStep createBlackAndWhite() {
return new BlackAndWhite();
}
...
So, if you want to create a black and white image with inverted colors, you need to do this (pseude code)
bw = PreProcessingFactory.createBlackAndWhite();
inv = PreProcessingFactory.createInverter();
result = bw.startProcessing(result);
result = inv.startProcessing(result);
In my mind, this is a good design since the implementation details are hidden and new processing steps can easily be add by adding a new class.
The other approach which was in my mind was to create a single class with every step as a static method – however, I’ve got dislike of ‘static’
Do you have any suggestions to improve the code?
3
You are consufing terms.
The problem is not Factory Class vs Static Class, but a method for each process vs a class for each process.
The “a class for each process” method that you seem to be considering has many advantages, mainly flexibility and scalability.
But in order to implement that, you will have to use some kind of Creational Pattern to help you keep your code decoupled from the concrete classes.
The Factory class approach you are using is OK.
The order or the way you call those processes is a matter of Behavioral Patterns like Chain of Responsability, Iterator, State, Command etc. These behavioral patterns are used for decoupling the requestor ( the class that needs to call a process ) from the concrete class that actually does the work.
2
I’ll make the following assumptions about your process:
- One step does not need to know about the inner workings of another step.
- Order of these steps is significant (you don’t obtain the same results performing these operations in the opposite order).
In this situation, I can only really tell you what I would do. I would avoid Factory pattern, simply because it implies you must have a factory method for every type of phase, and if you wanted to invent new stages, you’d also have to modify the factory as well. Plus while it may be manageable in bite-sized pieces, if you had to deal with 50 phases, even if it didn’t change often, it would still be very messy. Not to mention that factory would seem to imply that order is irrelevant. It makes you think that you can make any number of AbstractPreProcessingSteps and apply them in however order you wish. Even if that is not the case, I think it is generally better to present an interface which does not mislead a programmer.
Consider the following alternative pattern: Chain of responsibility
Suppose you had class AbstractProcessor as such:
public abstract class AbstractProcessor {
protected AbstractProcessor step;
public void setNextStep(AbstractProcessor step) {
this.step = step;
}
public abstract BufferedImage process(BufferedImage input);
}
Then you could have your steps as such:
public class ColorInverter extends AbstractProcessor {
public ColorInverter(AbstractProcessor step) {
setNextStep(step);
}
public BufferedImage process(BufferedImage input) {
// Perform color inverter process here
return step != null ? step.process(output) : output;
}
}
public class Rescaler extends AbstractProcessor {
public Rescaler(AbstractProcessor step) {
setNextStep(step);
}
public BufferedImage process(BufferedImage input) {
// Perform rescalar process here
return step != null ? step.process(output) : output;
}
}
Just like this, it is beautiful in its simplicity, however, you could override ordering for steps which must be performed at the end simply by calling “step.process(input)” and performing some operation on the returned result.
Also, if there were specific phases, you could override AbstractProcessor‘s setNextStep to require a certain type of AbstractProcessor to guarantee that certain phases are achieved. Take for example the following:
public abstract class RasteringProcessingStep extends AbstractProcessor {
public void setNextStep(RasteringProcessingStep step) {
super.setNextStep(step);
}
public void setNextStep(PostProcessingStep step) {
super.setNextStep(step);
}
}
In this way, you ensure that any step implementing RasteringProcessingStep must either be followed by another RasteringProcessingStep or it must be followed by a PostProcessingStep which also derives from AbstractPreProcessingStep.
To use it, you would see something like:
AbstractProcessor processor = new ColorInverter(new Rescaler(null));
File image = new File("strawberry.jpg");
BufferedImage output = processor.process(ImageIO.read(image));
Please let me know what you think about this approach.
2