Best practices about creating a generic object dictionary in C#? Is this bad?

Question

For clarity I am using C# 3.5/Asp.Net MVC 2

Here is what I have done: I wanted the ability to add/remove functionality to an object at run-time. So I simply added a generic object dictionary to my class like this:

public Dictionary<int, object> Components { get; set; }

Then I can add/remove any kind of .Net object into this dictionary at run-time. To insert an object I do something like this:

var tag = new Tag();
myObject.Components.Add((int)Types.Components.Tag, tag);

Then to retrieve I just do this:

if(myObject.Components.ContainsKey((int)Types.Components.Tag))
{    
    var tag = myObject.Components[(int)Types.Components.Tag] as Tag;
    if(tag != null) { //do stuff }
}

Somehow I feel sneaky doing this. It works okay, but I am wondering what you guys think about it as a best practice.

Thanks for your input, Daniel

Answer 1

I decided to abandon this implementation. It smells bad to me.

Instead I will accomplish what I need to do in a ViewModel class that lives in the application layer. The ViewModel will act as an aggregate for the domain model and all the other components that do not know about each other, but have a relationship under the domain model.

Answer 2

Because it's a dictionary, and you're using the key to imply the type of the object, you could only have one object of each type. Why not just have a single member of each type instead? Then you can just check whether it is null instead of worrying about enumerations and dictionaries and casting. It would also perform better. In this case, the simplest solution is the best practice, in my opinion. I would also document the property so that users of your class know that they can set it to add functionality.

The property:

public Tag TagComponent { get; set; }

Adding the component:

myObject.TagComponent = new Tag();

Usage:

if (TagComponent != null)
{
  //do stuff
}

The main problem with your idea is that it robs your class of its static definition, which will make users constantly guess at what its capabilities and components are at runtime.

Answer 3

I would something like this instead:

public class CoolClassName
{
    private Dictionary<Type, object> _items = new Dictionary<Type, object>();


    public T Get<T>()
    {
        T value;
        if (_items.TryGetValue(typeof(T), out value)
           return (T)value;

        return null;
    }

    public void Set<T>(T value)
    {
        _items[typeof(T)] = value;
    }
}

Usage:

coolClass.Set<IUser>(new User{FirstName = "Jonas"});
var user = coolClass.Get<IUser>();

It's a bit more type safe.

If you are looking for different solution, please describe in more detail what you want to do.