3
\$\begingroup\$

I have the following manager class that handles access and caching attributes from classes. I am a bit worried about the caching mechanism.

Is there a better way of handling it?

public static class AttributeManager
    {
        private static Dictionary<Type, Attribute> _cache = new Dictionary<Type, Attribute>();

        public static AttributeType GetAttribute<AttributeType>(Type type)
            where AttributeType : Attribute
        {

            if (_cache.ContainsKey(type))
            {
                return (AttributeType)_cache[type];
            }

            var attr = Attribute.GetCustomAttribute(type, typeof(AttributeType));

            if (attr != null)
            {
                _cache.Add(type, attr);
                return (AttributeType)attr;
            }
            else
            {
                throw new ArgumentException($"The specified attribute: {nameof(AttributeType)} was not found.");
            }
        }
    }
\$\endgroup\$
1
  • \$\begingroup\$ Does your application really requires attribute caching? Have you measured its performance? Attribute caching is a rather rare operation - I wouldn't bother if it wasn't necessary - see: To cache or not to cache - GetCustomAttributes \$\endgroup\$
    – t3chb0t
    Jun 23, 2016 at 8:43

2 Answers 2

6
\$\begingroup\$

Let’s define an extension method on Type:

public static class AttributeCache
{
    public static IEnumerable<T> Attributes<T>(this Type type)
        where T : Attribute => Cache
            .GetOrAdd(type, t => t.GetCustomAttributes(true).OfType<Attribute>().ToArray())
            .OfType<T>();        

    static ConcurrentDictionary<Type, Attribute[]> Cache { get; } = 
        new ConcurrentDictionary<Type, Attribute[]>();
}

Demo:

foreach (var a in typeof(MyCass).Attributes<TestAttribute>())
    Console.WriteLine(a.MyProp);
\$\endgroup\$
2
\$\begingroup\$

A type may contain more that one attributes... therefore it is not sufficient to use the type as key. It must be a combination of type and attribute type.


The static variable _cache should be readonly.


The following can be realized with just one lookup:

// two lookups:
if (_cache.ContainsKey(type))
{
    return (AttributeType)_cache[type];
}

// one lookup:
Attribute attribute;
if (_cache.TryGetValue(type, out attribute)
{
    return (AttributeType)attribute;
}

Consider to return null if the attribute is not available. However, you should not cast ((AttributeType)_cache[type];) the cached value. Use either _cache[type] as AttributeType if you want to return null or throw a custom exception - otherwise an InvalidCasteException will be thrown.

Example that produces an InvalidCastException:

void Main()
{
    var attributeA = AttributeManager.GetAttribute<AAttribute>(typeof(MyCass));
    // thrwows an InvalidCastException
    var attributeB = AttributeManager.GetAttribute<BAttribute>(typeof(MyCass));
}

[AAttribute("test")]
[BAttribute("test")]
public class MyCass
{
}

public class BAttribute : Attribute
{
    public BAttribute(string name)
    {
        Name = name;
    }
    public string Name { get; set; }
}

public class AAttribute : Attribute
{
    public AAttribute(string name)
    {
        Name = name;
    }
    public string Name { get; set;}
}

public static class AttributeManager
{
    private static Dictionary<Type, Attribute> _cache = new Dictionary<Type, Attribute>();

    public static AttributeType GetAttribute<AttributeType>(Type type)
        where AttributeType : Attribute
    {
        Attribute attribute;
        if (_cache.TryGetValue(type, out attribute))
        {
            return (AttributeType)attribute;
        }

        var attr = Attribute.GetCustomAttribute(type, typeof(AttributeType));

        if (attr != null)
        {
            _cache.Add(type, attr);
            return (AttributeType)attr;
        }
        else
        {
            throw new ArgumentException($"The specified attribute: {nameof(AttributeType)} was not found.");
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.