Frage

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?

War es hilfreich?

Lösung

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.

Andere Tipps

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.

Properties are designed for things that "look like" fields, such as the location of an object.

A property that returns a new instance every time you get it is very poor design.

You should use a method instead.

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top