HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Is this a good implementation of the Factory Pattern?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisthefactorygoodimplementationpattern

Problem

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 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:

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