Should a factory method of a custom container return the newly created instance?

Question

I have a custom collection Doohickeys indexed by keys. This collection has a factory method createDoohickey(key) and an accessor Doohickey(key). Should createDoohickey(key) return the new object or should it return void?

In the first case, I would use it like this

myDoohickey = doohickeys.createDoohickey(key);
doStuff(myDoohickey);

in the other case like this

doohickeys.createDoohickey(key);
doStuff(doohickeys(key));

Which one would you consider preferable, and why?

Answer 1

I'm not sure the container should have a factory method to create the object it contains - good seperation of concerns would be to seperate object creation from the container. Therefore, I think you should have something like this:

Doohickey myDookickey = new Doohickey();
doohickys.Add(key, myDookickey);
doStuff(myDookickey); // or
doStuff(doohickeys(key));

You are then free to use a seperate object factory or instantiate Doohickey directly. This makes it easier to unit test the container with mock Doohickey objects.

If you must have the factory method, I would return the Doohickey, because it is probably cheaper to ignore the return value (assuming a reference/pointer is returned) when it is not needed than to use the index to retreive it when it is.

Answer 2

Yes, a Factory should return the newly created instance. However like Alex pointed out you could seperate the Repository and the Factory. I would most likely design the factory to have a reference to the repository. I don't think it would be the end of the world of you combined the two into one class just as long as the factory method actually returns the newly created instance.

Answer 3

The example above embeds the factory method in the container class. I agree with willcodejavaforfood, and this may or may not be a good idea over the longer term. The issue is whether the reduced coupling of splitting the factory into its own class is worth the extra cost.

As far as why a factory method should return the newly created instance, it is preferable to decouple creation from containment. Some users of the class might want one but not the other. Also, when you couple creation and containment you can no longer (unit) test creation without also testing containment.

Answer 4

Actually, I think the best answer is from a colleague of mine. He suggested to have createDoohickey(key) indeed return the Doohickey; and, additionally, to call it getDoohickey(key) instead.