Is this a correct way to use Factory Pattern?

Question

I have an abstract class which implements the 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:

ProjectDataCreatorFactory factory = new ProjectDataCreatorFactory(p);
String s = factory.createFromString("name").getDataString();

Question

Is this the (or a) correct way to use the Factory Pattern? Is the approach with an interface and an abstract class a good design?

Answer 1

Some design considerations:

• 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.

• sk2212: Thanks for your considerations! I did not use "enums" before but it seems that this is a good way to handle with type constants.
• Eric P.: Enums are the most under-used, yet one of the more powerful Java features. They can be really handy for these types of things: docs.oracle.com/javase/tutorial/java/javaOO/enum.html

Answer 2

Note that there isn't a so called factory pattern, there is factory method and there is abstract factory.

I would create a separate factory class for ProjectDateCreator.CREATION and ProjectDateCreation.UPDATED. To answer your original question, never be afraid of interfaces and abstract classes. Interfaces are good for you.

Besides an abstract ProjectDateCreator class I would make each factory a singleton and if you know the value of createFromString's String argument compile-time then you can replace it with createURLCreator, createProjectStatusCreator etc. methods. That would make the code a bit more readable (yes, I know, this createFromString method is a small one, but we software developers are even too lazy for that). The IDataCreator --> AbstractCreator --> ProjectManagerCreator inheritance chain is good, no composition is needed here, the abstract class extension is a good approach. I would add that in an abstract class it's not necessary to include unimplemented interface methods.