patternjavaModerate
Is this a good implementation of the Factory Pattern?
Viewed 0 times
thisthefactorygoodimplementationpattern
Problem
Suppose I have this code:
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?
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 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?
Solution
Two things strike me:
Combining these:
Then to use:
Or:
EDIT: Note that here, if you want to introduce a new type which isn't stateless, it could override the
Admittedly this would break code which assumed all implementations were stateless...
- 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...
Code Snippets
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;
}
}BaseType type = EXTENDED_TYPEA.getBaseType();public void doSomething(BaseTypesValue baseTypeValue) {
baseTypeValue.getBaseType().someCallOnTheBaseType();
}EXTENDED_TYPEC(null) {
@Override public BaseType getBaseType() {
return new ExtendedTypeC();
}
};Context
StackExchange Code Review Q#11603, answer score: 11
Revisions (0)
No revisions yet.