Attribute container

I made this really simple "attribute container" class that allows you to store arbitrary data to an object at will, sorta like in python how you can just assign attributes to arbitrary objects, but in C#. My gut is telling me it's a terrible idea, but I just want to get a group opinion.

public class AttributeContainer
    {

        private Dictionary attributes = new Dictionary();

        public T GetAttr(string name)
        {
            return ((Dictionary)attributes[typeof(T)])[name];
        }

        public void SetAttr(string name, T val)
        {
            var t = typeof(T);
            if (attributes.ContainsKey(t))
                ((Dictionary)attributes[t])[name] = val;
            else
                attributes[t] = new Dictionary() { { name, val } };
        }

    }

Then other simple classes can inherit from this.

I only really have one reason for making it. I have a system where there are a lot of classes that have a common operation Foo that act on each other, and return some data. The problem is that while the operation is common, the data they may return isn't. I feel like it would be a waste of time to create 30 different classes with different information Foo can return when I can do something like this instead.

What do you think? Is it a stupid idea?

Here's an example where the Foo operations are methods that test for collisions between different geometric objects and the return values are different geometric results (simplified of course):

```
public class CollisionResult : AttributeContainer
{
ICollidable A, B; // The two geometries involved in the collision.
public bool Colliding { get; private set; }
public CollisionResult(ICollidable a, ICollidable b, bool colliding)
{
A = a;
B = b;
Colliding = colliding;
}
}

public static class CollisionChecker
{
public static CollisionBetween(Point a, Point b)
{
bool result;
// Some collision check...

I have used a similiar approach at a time I needed it and in my opinion it is a valid way. That beeing said, let us take a look at the way you have implemented this.

public void SetAttr(string name, T val)
{
    var t = typeof(T);
    if (attributes.ContainsKey(t))
        ((Dictionary)attributes[t])[name] = val;
    else
        attributes[t] = new Dictionary() { { name, val } };
}

The first improvement you should make is to avoid the call to the ContainsKey() method. Performance wise it is better to use TryGetValue() over ContainsKey().

See also: what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem

The next I would always do is use brecaes {} for single commend if..else..else if statements to avoid stupid bugs.

Implementing the above would then lead to

public void SetAttr(string name, T val)
{
    var t = typeof(T);

    object obj;

    if (attributes.TryGetValue(t, out obj))
    {
        Dictionary dictionary = obj as Dictionary;
        dictionary[name] = val;
    }
    else
    {
        attributes[t] = new Dictionary() { { name, val } };
    }
}

which makes it more clear what the object in the attributes dictionary is.

What should happen if you try to retrieve a value form the container which isn't present ? How about adding something like a TryGetValue() method to the container class.

Something like so

public bool TryGetAtrribute(string name, out T val)
{
    object obj;

    if (attributes.TryGetValue(typeof(T), out obj))
    {
        Dictionary dictionary = obj as Dictionary;
        if (dictionary != null)
        {
            val = dictionary[name];
            return true;
        }
    }

    val = default(T);
    return false;
}

In this way it is more obvious for a different developer that the GetAttr() method can throw an exception.

Talking about exception, I would consider to throw an AttributeNotFoundException for the case that the desired value isn't in the container.

StackExchange Code Review Q#95165, answer score: 5