Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Use a property as factory method

I have a base class Base which needs to create instances of another type TRequired, however, only derived classes from Base know how to construct those.

Is it bad style to use an abstract property as factory method? e.g.

protected abstract TRequired NewTRequired { get; }

Should I use a method for some reason? Is there a guide why I should/shouldn't use a property here?

like image 484
D.R. Avatar asked Dec 02 '22 22:12

D.R.


2 Answers

You should definitely use a method because accessing this member does something. Calling a method is a good way to let the code speak for itself in that regard.

Or, if you prefer another perspective: two subsequent accesses of the member will return different results. A good rule of thumb is to use a method whenever this is the case, in order to not violate the principle of least astonishment.

This looks like it's reading the result of a variable, even though if you know that NewTRequired is a property (as opposed to a field) you also know that in reality it's running arbitrary code:

var prototype = Factory.NewTRequired;

I have deliberately put the result into a variable called prototype in order to better show that even an informed reader of this code can be easily thrown off: it would not be unreasonable to see this and think "right, so NewTRequired is the prototype object for X". That reader would certainly be astonished by the result of code like this:

var eq = object.ReferenceEquals(prototype, Factory.NewTRequired);

Contrast this with a factory method. Now this code might give off a slight smell:

// hmmm... are we actually using this as a prototype?
// because it sure looks like an instance created just for the occasion.
var prototype = Factory.NewTRequired();

And this code will never astonish you:

// obviously should be false, the code screams "I am creating new instances!"
var eq = object.ReferenceEquals(Factory.NewTRequired(), Factory.NewTRequired());

A famous example of where this rule really should have been followed but was not is the DateTime.Now property.

like image 150
Jon Avatar answered Dec 14 '22 22:12

Jon


I would recommend a method instead:

protected abstract TRequired CreateRequired();

Creation implies "work" occurring. This is a better fit for a method vs. a property, as a property getter implies something that will typically be returned quickly without executing much code.

Even your question title "property as factory method" implies that a factory method should be a method.

like image 42
Reed Copsey Avatar answered Dec 15 '22 00:12

Reed Copsey