0

I have created factory class and i wonder which is better way to implement it. option 1

public class Factory { private IProperty prop; public IDoc doc; public Factory(int version) { switch (version) { case '1': prop = new Prop(); doc = new Docu(); ... case '2': prop = new Prop1(); doc = new Docu1(); ... } } public IProperty getProperty() { return this.prop; } public IDoc getDoc() { return this.doc; } }

My question is if to do it like that i.e. define member with the interface type and to to switch on the constructor or for every get method to use switch statement instead on the constructor, so in the constructor i will just get the version and save it on class member and than for instance use like

public IProperty getProperty() { switch (version) { case '1': prop = new Prop(); case '2': prop = new Prop1(); ...

So what is the better way, or any other idea?

Improve this question
3
  • Your first version will not even compile. Your switch is outside any method. Its not allowed. And as per your question, 2nd option is always a better way to go. Let the getProperty method decide how it builds the property. Commented Nov 15, 2012 at 7:22
  • One more question, why your factory have non-static reference instead of static references? Commented Nov 15, 2012 at 7:29
  • Either 1st or 2nd way is possible - they have minor difference. In the 1st case IProperty/IDoc instances returned by the same instance of Factory will always be the same, while in 2nd case there is still a possibility to change instances returned by Factory instance during it's lifetime. Though presence of switch statement clearly indicates that polymorphism can (and actually should) be used instead of it - please refer to bobah answer which describes how it is done. Commented Nov 15, 2012 at 8:12

5 Answers 5

Reset to default
5

The most clean way is to expose what you do as two separate factories giving them a common abstract base or a reusable policy argument if they have anything to share. One factory type should only create one type of particular object (say only plastic tools). Factory public configuration normally only holds properties required to create objects (contacts of suppliers, patents) or static properties of objects being created (type of plastic, let's say), but not type/class of objects.

Also, something that is a storage of long lived objects like in your example #1 should probably be called "context", not "factory".

Code example below.

public interface IFactory { IDoc createDoc(); IProp createProp(); } public class Type1Factory implements IFactory { @Override public IDoc createDoc() { return new Doc1(); } @Override public IProp createProp() { return new Prop1(); } }
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Just wanted to write something similar but noticed this answer, thus adding +1 :) The described way is actually called Abstract Factory pattern which one can easily google and which has a lot of benefits over other implementations.
+1 for this meaningful line ** One factory type should only create one type of particular object. **
0

  • First of all, your Factory should ideally have static references instead of non-static one. And it should have a static method to create / get the appropriate instance.

  • Secondly, its better to have two different factories for different types.

  • Thirdly, I would name your method as createProperty, rather than getPropertyObject, because that method is not returning the already created instance, rather its creating one.

Of course, getPropertyObject('1'), seems like it is fetching a property for that version from a persistence storage, which is not what it is doing. It is rather creating an instance based on version.

(NOTE: - Name of static factory methods are important. They are amongst one of the advantages, they have over constructors. Since with a name, you can guess what exactly that factory method does)

Having said that, I would say, that the 2nd option would be better with all those changes. Let the createProperty method decide, how it wants to create the instance.

So, I would modify your code like this: -

public class PropertyFactory { private static IProperty prop; public static createProperty(char version) { switch (version) { case '1': prop = new Prop(); break; // Don't forget a `break` here. case '2': prop = new Prop1(); break; default: // do have a default case prop = null; } return prop; } }

Similarly, you can create a DocumentFactory to create a document object. Name the method: - createDocument(char version)

Improve this answer

Comments

0

Your second way is always better since every caller of your get* methods will receive a new instance of the object. If you create the two objects in the constructor, you will have to handle object-sharing issues (even more if you use these objects in different threads).

Improve this answer

Comments

0

The idea of creating a factory is much better in your first version than second one.

Ideally, it shouldn't be the constructor of factory class but a static method.

public class Factory { public static IProperty getPropertyObject(char version) { switch (version) { case '1': return new Prop(); case '2' return new Prop1(); } } public static IDoc getDocObject(char version) { switch (version) { case '1': return new Doc(); case '2' return new Doc1(); } } }
Improve this answer

1 Comment

A static method would reduce testability as it would introduce a hard reference on a specific class.
0

It depends on your situation.

The first option suggests that your IProperty and IDoc have different versions but for each version of one you have a corresponding version of the other.

While the second option suggests that the versions of those could be independent of each other.

Improve this answer

Comments