Hello. I ask your advice here on what matter. I have a class hierarchy

enum SomeEnum { First, Second } abstract class BaseClass { private SomeEnum ActionId { get; set; } protected BaseClass(SomeEnum actionId) { ActionId = actionId; } public static T GetById<T>(SomeEnum actionId) where T : BaseClass { switch(actionId) { case SomeEnum.First: return new DerivedFirst(actionId) as T; case SomeEnum.Second: return new DerivedSecond(actionId) as T; default: throw new ArgumentException(); } } } class DerivedFirst : BaseClass { public DerivedFirst(SomeEnum actionId) : base(actionId) { } } class DerivedSecond : BaseClass { public DerivedSecond(SomeEnum actionId) : base(actionId) { } }

The problem is that in the abstract class BaseClass there is a static method that returns an instance of the class DerivedFirst or DerivedSecond depending on the type of the actionId parameter. It seems to me that I wrote a bad decision. Maybe you can tell something better, nothing comes to mind ((

  • one
    Good day! Based on your code, I understand that you are trying to implement the design pattern "factory method". Here is a link to Wikipedia: ru.wikipedia.org/wiki/… . I hope to help!) - progzdeveloper

3 answers 3

(Update the answer in connection with the changed condition.)

 class ActionFactory { static Dictionary<SomeEnum, Func<BaseClass>> activators = new Dictionary<SomeEnum, Func<BaseClass>>() { { SomeEnum.First, () => new DerivedFirst(SomeEnum.First) }, { SomeEnum.Second, () => new DerivedSecond(SomeEnum.Second) } }; public static BaseClass Create(SomeEnum actionId) { Func<BaseClass> activator; if (!activators.TryGetValue(actionId, out activator)) throw new ArgumentException("Unsupported action id", "actionId"); return activator(); } }

There is no point in doing the Create generic function, since it requires knowledge of the type of the return value at the compilation stage, and if this type is known, you can directly call the constructor.

  • Yes, I understand, but I just would like not to duplicate this method in each successor, but to return new T (id) depending on the type of T - JuniorTwo
  • @JuniorTwo: Uh ... You're not complaining that you have to implement a virtual function in every child class? If your GetById function GetById always so simple, you don’t need it, use the constructor. If it is more cunning than just calling the constructor, storing all the knowledge about how to return an object by id wrong in the base class for about the same reason that it uses virtual functions, and not a bold function with a long switch in the base classroom. - VladD
  • Sorry, I did not quite correctly formulated the question initially. Reformulated now - JuniorTwo
  • @JuniorTwo: (1) In this version, your code will not compile at all, you need constructors. (2) The last comment is still applicable. In GetById you have a potentially huge switch on possible types, this is bad, information on how to construct a class should be inside the class itself. - VladD pm
  • I'm sorry, I'm stupid, apparently - I could write an example only from the third attempt. - JuniorTwo

I do not know why no one paid special attention to the presence of the "spirit" of the Factory pattern in the program code. If you do not like your code, then perhaps you are not mistaken. I decided to rewrite your code in the semantics of the abstract factory:

 ... enum SomeEnum{First,Second} abstract class Factory { private SomeEnum ActionId; protected Factory(SomeEnum actionId) { ActionId = actionId; } public static Factory GetById(SomeEnum actionId) { switch (actionId) { case SomeEnum.First: return new FactoryFirst(actionId); case SomeEnum.Second: return new FactorySecond(actionId); default: return new FactoryException(actionId); } } } /* Здесь мы определяем сущности "фабрики"( что она может выпускать ) */ class FactoryFirst : Factory { public FactoryFirst(SomeEnum actionId) : base(actionId) { } } class FactorySecond : Factory { public FactorySecond(SomeEnum actionId) : base(actionId) { } } class FactoryException : Factory { public FactoryException(SomeEnum actionId) : base(actionId) { MessageBox.Show("Not Valid ActionId!"); } } ...

Using:

 ... Factory factoryFirst = Factory.GetById(SomeEnum.First); Factory factorySecond = Factory.GetById(SomeEnum.First); ...

Obviously, the entire interface of the "factory objects" should now be described in the parent abstract class.

  • messagebox in exception constructor? You are not confused levels of model and presentation? - VladD 5:44 pm
  • I'm sorry, but how does your version differ from what is given in my question? Well, except for the modified class names? - JuniorTwo
  • @VladD, this is just an example. You can change as you wish (MessageBox, if necessary, remove) :) - AseN

First, the static method is likely to be declared as returning the base class. Otherwise, using the code still needs to know what type of object it wants to receive (and indicate this in the generic parameter), which makes the presence of the method not very meaningful - why not just call the necessary constructor?

On the other hand, this option is also possible if this method does some significant initialization work (in which its goal is to initialize, not to create), in order to avoid the need for additional type conversion. However, in this case, the enum parameter is redundant, since a generic type is sufficient.

By the way, in the latter case, I would transfer the already created object to the method, initialize it and return it. Then the method will remain generic, but the type can already be output by the compiler.

Further, about the pattern, the factories have already said here, although I don’t know if you need it. But there is another thought - what is sometimes called the smart enum. There is a base class and its previously known descendants, which are used inside the class, and outside of them only methods of the base class are required. In this case, it makes sense to put these classes inside the base and make it private. Again, the creation method must be declared as returning the base class.