9
votes
\$\begingroup\$

Suppose I have this code:

public interface BaseType {
         public void doSomething();
    }

public class ExtendedTypeA implements BaseType {
//No Instance Variables

    @Override
    public void doSomething() {
        //really do something
    }
}

public class ExtendedTypeB implements BaseType {
//No instance variables

    @Override
    public void doSomething() {
        //really do something, but different
    }
}

public enum BaseTypesValues { EXTENDED_TYPEA, EXTENDED_TYPEB }

public BaseTypeFactoryStandard {

    public BaseType getBaseType(BaseTypesValues baseTypeValue) {
        switch(baseTypes) {
            case BaseTypesValues.EXTENDED_TYPEA:
                return new ExtendedTypeA();
            case BaseTypesValues.EXTENDED_TYPEB:
                return new ExtendedTypeB();
            default:
                throw new NoSuchTypeException();
        }
    }
}


public BaseTypeFactoryMyWay {
    public static final Map<BaseTypesValues, BaseType> factoryMap = new HashMap<//...

    static {
        factoryMap.put(BaseTypesValues.EXTENDED_TYPEA, new ExtendedTypeA());
        factoryMap.put(BaseTypesValues.EXTENDED_TYPEB, new ExtendedTypeB());
    }

    public BaseType getBaseType(BaseTypesValues baseTypeValue) {
        return factoryMap.get(baseTypeValue);
    }

}

Is the last class a good/valid implementation of the factory pattern? Take into consideration the fact that classes that implement the BaseType DO NOT have a state (no non-final instance variables); this means that the objects are lightweight.

Also, can it be implemented in this way (using a map) for the general case (in which classes have state).

Or is this a dumb way in either cases?

\$\endgroup\$

2 Answers 2

11
votes
\$\begingroup\$

Two things strike me:

  • There's no need to have a separate factory class when you could put the functionality into the enum, unless you expect to have other factory implementations
  • As you say, if the classes are stateless, there's no need to create a new instance on each call. (I've only just spotted your factory map class, which effectively does something like this, but there's no need to use a map.)

Combining these:

public enum BaseTypesValues {
    EXTENDED_TYPEA(new ExtendedTypeA()),
    EXTENDED_TYPEB(new ExtendedTypeB());

    private final BaseType instance;

    private BaseTypesValues(BaseType instance) {
        this.instance = instance;
    }

    public BaseType getBaseType() {
        return instance;
    }
}

Then to use:

BaseType type = EXTENDED_TYPEA.getBaseType();

Or:

public void doSomething(BaseTypesValue baseTypeValue) {
    baseTypeValue.getBaseType().someCallOnTheBaseType();
}

EDIT: Note that here, if you want to introduce a new type which isn't stateless, it could override the getBaseType method:

EXTENDED_TYPEC(null) {
    @Override public BaseType getBaseType() {
        return new ExtendedTypeC();
    }
};

Admittedly this would break code which assumed all implementations were stateless...

\$\endgroup\$
4
  • \$\begingroup\$ His implementation (BaseFactoryMyWay) actually has the property of not creating a new instance on each call. \$\endgroup\$
    – Romain
    May 8, 2012 at 14:51
  • \$\begingroup\$ @Romain: Yes, I'd missed that before. Have edited. \$\endgroup\$
    – Jon Skeet
    May 8, 2012 at 14:52
  • \$\begingroup\$ and if classes aren't stateless, it's very likely you can't just put one instance of them into a Map and return the same instance on each call. \$\endgroup\$
    – Victor Sorokin
    May 8, 2012 at 14:52
  • \$\begingroup\$ I have provided the general/ideal case. The case in which I am actually working on actually depends on a String rather than an enum. Anyway, this implementation is really nice; thanks for it. \$\endgroup\$
    – m3th0dman
    May 8, 2012 at 14:54
1
vote
\$\begingroup\$

What you're doing is not a Factory anymore, some people call it a "multiton", it's a multi-instance variation around the singleton pattern.

And yes, you're doing it right.

That said, if you only have a single implementation, you don't need the factory at all. Like Jon Skeet mentions, you should move the behavior into the Enum.

\$\endgroup\$
1
  • \$\begingroup\$ In BaseTypeFactoryMyWay, yes, that's a multiton. But in the BaseTypeFactoryStandard, that's a factory. You don't make this distinction very clear in your answer. \$\endgroup\$ Feb 4, 2014 at 11:14