2

Greetings everyone!

I'll try to make my problem simple: I have an enum to select which ObjType I should use (ObjTypeA and ObjTypeB both inherits from ObjType). So I created a method to extend the given enum, in order to return a new instance according to the selected property in the enum, like follows in the code. I think it works more or less like a factory design pattern. So far so good, but eventually, like in the class MyClass, I may attempt to create n instances of ObjTypeA or ObjTypeB, but I'll have to face the if statement everytime I call the GetObjTypeInstance() method. So:

  • Can an enum return an instance, something like: public enum EObjType { ObjTypeA = new ObjTypeA(), ObjTypeB = new ObjTypeB() }? Actually, it'd be better to append some GetInstance() method to the ObjTypeA and to the ObjTypeB options in the enum. If there's a way to do this, how can I do it? Doing this I'd avoid those if statements every while step.
  • Is there any other (and better) way to this this (if you understood my problem...)? How?

Thanks in advance!

Follow the example code:

public static class EObjTypeExt
{
    public static ObjType GetObjTypeInstance(this EObjType ot)
    {
        if (ot == EObjType.ObjTypeA)
        {
            return new ObjTypeA();
        }
        else if (ot == EObjType.ObjTypeB)
        {
            return new ObjTypeB();
        }
        throw new ArgumentOutOfRangeException("unrecognized type!");
    }
}

public enum EObjType { ObjTypeA, ObjTypeB }

public class MyClass
{
    ObjType[] obj { get; set; }

    public MyClass(EObjType otEnum, int n)
    {
        this.obj = new ObjType[n];
        int i = 0;
        while (i < n)
        {
            this.obj[i] = otEnum.GetObjTypeInstance();
            i++;
        }
    }
}
Danny Varod
  • 17,324
  • 5
  • 69
  • 111
Girardi
  • 2,734
  • 3
  • 35
  • 50
  • Why do you want to avoid the if-statements in the sample code? – Lasse V. Karlsen May 17 '11 at 22:17
  • If `n` is big enough (and it generally is in my problem), then the code would become very slow. – Girardi May 17 '11 at 22:18
  • You mean the `if (ot == EObjType.ObjTypeA)` checks? They won't be significant at all next to the object allocations. – Rup May 17 '11 at 22:19
  • Girardi - "become very slow" : You're not going to write that many classes. – H H May 17 '11 at 22:23
  • @Rup yeah, maybe... I didn't test it, I'm wondering that a significative high `n` would pass through a significative amount of `if` statements. But actually, the `new ObjTypeA()` or B would not take too much time... – Girardi May 17 '11 at 22:23
  • Look here http://stackoverflow.com/questions/261663/can-we-define-implicit-conversions-of-enums-in-c/2949340#2949340 and here http://stackoverflow.com/questions/261663/can-we-define-implicit-conversions-of-enums-in-c/2949340#2949340 – sehe May 17 '11 at 22:44

7 Answers7

3

You'll have to byte this apple somewhere.

Maybe replace the if/elseif chain with switch statement, they work great with enums.

H H
  • 263,252
  • 30
  • 330
  • 514
  • For a small enum like this, though, I'd be surprised if a switch didn't just compile down to the two ifs. – Rup May 17 '11 at 22:20
  • A switch can also compile to a dictionary. Not our business/problem. The only criterion is readability. – H H May 17 '11 at 22:22
  • 1
    @Danny Varod: "...Still not clean code" Really? In what world do you live in where switch statements implicitly mean that the code is bad? How do you think factories work? – Ed S. May 17 '11 at 22:25
  • I don't understand why you would *have* to deal with this. It looks to me like a classic example for applying a replace conditional with polymorphism refactoring. That effectively eliminates the need for any if/switch logic. – Harry Steinhilber May 18 '11 at 02:34
1

You need to use a factory or other creational design pattern.

For instance, you could hold a dictionary from enum key to type value to get the desired class type using selected enum value. Then use reflection to create a new instance (object) of received type.

Initialize static dictionary's values using static constructor of factory class. You can enter the values manually or better yet, load possible values from a config file.

Danny Varod
  • 17,324
  • 5
  • 69
  • 111
  • Sorry, but I'm no expert, could you be more precise? Tks! – Girardi May 17 '11 at 22:19
  • About which part? Try Googling the stuff you didn't understand. – Danny Varod May 17 '11 at 22:21
  • 1
    A dictionary lookup and reflection? Isn't that all going to be slower than what he's already got? – Rup May 17 '11 at 22:22
  • If he preloads the assembly, or it is already in memory, then no. All that can be done at startup by a static constructor on factory class. – Danny Varod May 17 '11 at 22:25
  • @Danny: I don't know what you mean by "dictionary values" (I only know the Dictionary class). And I don't know how to create an instance by reflection, although I'd searched that, and some said that it is not good to do that... – Girardi May 17 '11 at 22:29
  • Dictionary of TKey = your enum type, TValue = System.Type. Example dict.Add(MyEnum.Key1, typeof(MyClass1)). - Standard dictionary class. – Danny Varod May 17 '11 at 22:34
1

I'm not sure that I'd really advocate this approach, but you could call the enum ToString() method, treat that as your class name and use reflection to instantiate an object of that type.

One advantage of this would be that you could reflect and get the type once, then call the constructor n times in your loop.

GalacticCowboy
  • 11,663
  • 2
  • 41
  • 66
  • You mean almost what @shsmith did, using: `var obj = Activator.CreateInstance(null, otEnum.ToString());` inside MyClass ? – Girardi May 17 '11 at 23:28
  • @Girardi - yeah, something like that. It's been about 2 years since I wrote any code like that, so I don't have any handy examples lying around. – GalacticCowboy May 18 '11 at 12:46
1

Instead of using an enum, I would use a class that looks like an enum:

public class EObjType {
    public static readonly EObjType ObjTypeA = new EObjType(() => (ObjType)(new ObjTypeA));
    public static readonly EObjType ObjTypeB = new EObjType(() => (ObjType)(new ObjTypeB));

    private readonly Func<ObjType> generator;
    private EObjType(Func<ObjType> generator) {
        this.generator = generator;
    }

    public ObjType GetInstanceOfObjType() {
        return generator();
    }
}

You can then use it exactly as you have been the enum.

EObjType otEnum = EObjType.ObjTypeA;
ObjType obj = otEnum.GetInstanceOfObjType();
Harry Steinhilber
  • 5,219
  • 1
  • 25
  • 23
  • Wouldn't that create an instance of both `ObjTypeA` and B every time I do `EObjType otEnum = EObjType.ObjTypeA` or B? – Girardi May 18 '11 at 00:00
  • No, it's using a delagate to defer creation of the `ObjType` instances until you call `GetInstanceOfObjType`. The two instances of `EObjType` will be created the first time `EObjType` is accessed, but no other instances will ever be created. Those `EObjType` instances can then create the appropriate `ObjType` without any `if`, `switch`, or `Dictionary`. – Harry Steinhilber May 18 '11 at 02:28
1

As Danny Varod points out, a dictionary mapping your enum values to their Types (or to functions that create those types) would allow you to avoid if statements. Since enum is really just an integer underneath, an array would be more memory and time efficient, but readability is probably most important here.

Neil
  • 7,227
  • 5
  • 42
  • 43
  • I agree, readability is the most important here. Dictionary performance should be close to Array's. – Danny Varod May 17 '11 at 22:38
  • @Danny: Dictionary and Array are actually quite different in performance, although they are both O(1) operations. A couple of years ago, when I tested, a dictionary read was on par with 200 method calls. This may have changed with the post 3.5 dictionary, and will definitely depend on other factors like the type of your key. All that said, this will only matter in a very tight loop. – Neil May 18 '11 at 19:38
1

You could create a factory that allows registration of functions that map to your enumeration, you could that use some sort of registration process to register your different enumerations

public class ObjectFactory
{
    private readonly Dictionary<MyObjectType, Func<MyObject>> _store = new Dictionary<MyObjectType, Func<MyObject>>();

    public void Register<T>(MyObjectType type) where T: MyObject, new()
    {            
        this.Register(type, () => new T());
    }

    public void Register(MyObjectType type, Func<MyObject> factory)
    {
        _store.Add(type, factory);
    }

    public MyObject CreateInstance(MyObjectType type)
    {
        Func<MyObject> factory;
        if(_store.TryGetValue(type, out factory))
        {
            return factory.Invoke();
        }
        return null;
    }
}

public enum MyObjectType { A, B }

public class MyObject {}

public class MyObjectA : MyObject {}
public class MyObjectB : MyObject {}

Usage as follows

var factory = new ObjectFactory();
factory.Register<MyObjectA>(MyObjectType.A);
factory.Register<MyObjectB>(MyObjectType.B);

var a = factory.CreateInstance(MyObjectType.A);
var b = factory.CreateInstance(MyObjectType.B);

Assert.IsInstanceOf(typeof(MyObjectA), a);
Assert.IsInstanceOf(typeof(MyObjectB), b);
Rohan West
  • 9,262
  • 3
  • 37
  • 64
  • Nice, but in what way is this an improvement? Readability? Extensibility (you still need those calls to Register) ? – H H May 17 '11 at 22:31
  • @Henk Good point... It only adds the ability to register mappings based on an external condition plus mappings can be defined in multiple places, bit like an IOC container – Rohan West May 17 '11 at 22:39
  • I think this solution is more or less like @Danny 's one, isn't it? – Girardi May 17 '11 at 23:04
  • @Girardi, Yep, just missing the static implementation of the object factory – Rohan West May 17 '11 at 23:09
1

You could use Activator.CreateInstance.

public class ObjType {}
public class ObjTypeA : ObjType {}
public class ObjTypeB : ObjType {}
public enum EObjType { ObjTypeA, ObjTypeB }

public static class EObjTypeExt
{
    public static ObjType GetObjTypeInstance( EObjType ot)
    {
        object o = Activator.CreateInstance(null,ot.ToString());
        return (ObjType)o;
    }
}
hsmiths
  • 1,257
  • 13
  • 17
  • It throws `TypeLoadException`, unless I write `Activator.CreateInstance(null, "Namespace." + ot.ToString())` where `Namespace` is the namespace where the class is located – Girardi May 17 '11 at 23:35
  • Even If I do that, it throws `InvalidCastException: Unable to cast object of type 'System.Runtime.Remoting.ObjectHandle' to type 'Namespace.ObjType'.` – Girardi May 17 '11 at 23:37