patternjavaMinor
Is this a correct way to use Factory Pattern?
Viewed 0 times
thisfactorywaycorrectusepattern
Problem
I have an abstract class which implements the
Then I have 5 concrete classes which extend
Factory class is implemented like this:
The factory class is called like this:
```
ProjectDataCreatorFa
IDataCreator interface:public interface IDataCreator
{
public String getDataString();
}
public abstract class AbstractCreator implements IDataCreator
{
protected IProject p;
public AbstractCreator(IProject p) {
this.p = p;
}
public abstract String getDataString();
}Then I have 5 concrete classes which extend
AbstractCreator and implement the getDataString() method:public class ProjectManagerCreator extends AbstractCreator
{
public ProjectManagerCreator(IProject p) {
super(p);
}
@Override
public String getDataString() {
if(p.getLead() != null) {
String lead = p.getLead().getName();
return lead;
}
return "";
}
}Factory class is implemented like this:
public class ProjectDataCreatorFactory
{
IProject p = null;
public ProjectDataCreatorFactory(IProject p) {
this.p = p;
}
/**
* Factory method which constructs a object from a given string value.
*
* @param column string of the column name TODO: Implement a interface which maps the strings into static variables
* @return object which implements the {@link IDataCreator} interface
*/
public IDataCreator createFromString(String column) {
IDataCreator creator = null;
if(column.equals("name")) {
return new URLCreator(p);
} else if(column.equals("status")) {
return new ProjectStatusCreator(p);
} else if(column.equals("manager")) {
return new ProjectManagerCreator(p);
} else if(column.equals("setupdate")) {
return new ProjectDateCreator(p, ProjectDateCreator.CREATION);
} else if(column.equals("updatedate")) {
return new ProjectDateCreator(p, ProjectDateCreator.UPDATED);
}
return creator;
}
}The factory class is called like this:
```
ProjectDataCreatorFa
Solution
Some design considerations:
-
Returning Null: Your factory method
-
Date Abstraction:
-
Interface Naming: Interfaces in Java aren't typically prefixed with an
-
Column Strings: Your
-
Returning Null: Your factory method
createFromString returns a null creator if the column is unknown. It's good practice to note this in the javadoc or perhaps throw an IllegalArgumentException if a column you can't handle is passed in.-
Date Abstraction:
ProjectDateCreatorlooks like it could use either its own factory or two separate implementations based on the second constructor argument. That class isn't included here, but it appears CREATION and UPDATED are some sort of public static final type constants. Those would be better expressed as an enum:// A factory can use this instead!
public enum ProjectDateOption {
CREATION, UPDATED;
}-
Interface Naming: Interfaces in Java aren't typically prefixed with an
I, but if it's consistent through your code then that's a minor quibble.-
Column Strings: Your
TODO mentions this, but the column strings could also be placed into an enum, and then the checking could occur there.Code Snippets
// A factory can use this instead!
public enum ProjectDateOption {
CREATION, UPDATED;
}Context
StackExchange Code Review Q#24031, answer score: 3
Revisions (0)
No revisions yet.