Attribute container

Question

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<Type, object> attributes = new Dictionary<Type, object>();
 public T GetAttr<T>(string name)
 {
 return ((Dictionary<string, T>)attributes[typeof(T)])[name];
 }
 public void SetAttr<T>(string name, T val)
 {
 var t = typeof(T);
 if (attributes.ContainsKey(t))
 ((Dictionary<string, T>)attributes[t])[name] = val;
 else
 attributes[t] = new Dictionary<string, T>() { { 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...
 return new CollisionResult(a, b, result);
 }
 public static CollisionBetween(Line a, Ray b);
 {
 bool result;
 // again...
 var c_res = new CollisionResult(a, b, result);
 if (!result)
 return c_res;
 c_res.SetAttr<Vector2>("IntersectionPoint", poi);
 c_res.SetAttr<Quadrant>("RelIntersectionQuadrant", rel_intquad); // the quadrant relative to `a` of intersection.
 // other attributes specific to this collision type...
 return c_res;
 }
 public static CollisionBetween(Ellipse a, Rect b)
 {
 bool result;
 // again...
 var c_res = new CollisionResult(a, b, result);
 if (!result)
 return c_res;
 c_res.SetAttr<Vector2[]>("IntersectionPoints", points);
 c_res.SetAttr<double>("OverlappingArea", overlap);
 c_res.SetAttr<bool>("Crossing", crossing); // crossing occurs when the bounding rects form a plus shape.
 // other attributes specific to this collision type...
 return c_res;
 }
 public static CollisionBetween(ConcavePolygon a, ConcavePolygon b)
 {
 bool result;
 // Perform Separating Axis Theorem algorithm...
 var c_res = new CollisionResult(a, b, result);
 if (!result)
 return c_res;
 c_res.SetAttr<Vector2[]>("OverlappingAxes", overlapping_axes);
 c_res.SetAttr<Triangles[]>("IntersectingTriangles", intersecting_triangles); // the triangles are from the triangulations of each polygon
 c_res.SetAttr<Vector2>("CollisionResolutionAxis", col_resolution_axis);
 // other attributes specific to this collision type...
 return c_res;
 }
}

In the real application, there are 12 different types of geometries, which (if all collision results were different) would require a maximum of 78 different subclasses of CollisionResult. I'm sure some clever inheritance tree could reduce that value substantially, but it still feels like a lot when you can just return one AttributeContainer.

• janos: Have you considered using parameterized types?

Answer 1

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<T>(string name, T val)
{
 var t = typeof(T);
 if (attributes.ContainsKey(t))
 ((Dictionary<string, T>)attributes[t])[name] = val;
 else
 attributes[t] = new Dictionary<string, T>() { { 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<T>(string name, T val)
{
 var t = typeof(T);
 object obj;
 if (attributes.TryGetValue(t, out obj))
 {
 Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
 dictionary[name] = val;
 }
 else
 {
 attributes[t] = new Dictionary<string, T>() { { 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<T>() method to the container class.

Something like so

public bool TryGetAtrribute<T>(string name, out T val)
{
 object obj;
 if (attributes.TryGetValue(typeof(T), out obj))
 {
 Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
 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<T>() 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.

• redman: While he knows that the value in the dictionary will always be of type Dictionary<string, T>, so use of the as operator will not result in a null, it always makes me twitch a bit to see the use of as without a null check. If you're certain it's going to be a valid cast and not null, why not just straight cast it?
• Heslacher: @redman Good point. Updated answer. About cast vs as I prefer the second.