Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Suppose we have an enum called "Planet" and it has a custom attribute of class "PlanetAttr", these methods will give you the attribute value for a given Planet value:

private static PlanetAttr GetAttr(Planet p)
{
    return (PlanetAttr)Attribute.GetCustomAttribute(ForValue(p), typeof(PlanetAttr));
}

private static MemberInfo ForValue(Planet p)
{
    return typeof(Planet).GetField(Enum.GetName(typeof(Planet), p));
}

But is this a good way to retrieve the value of a custom attribute from an enum? I am uncomfortable with the number of method calls involved. Is there a more efficient way, e.g. can it be done without looking up the enum constant's name?

This code is from a StackOverflow question.

using System;
using System.Reflection;

class PlanetAttr: Attribute
{
    internal PlanetAttr(double mass, double radius)
    {
        this.Mass = mass;
        this.Radius = radius;
    }
    public double Mass { get; private set; }
    public double Radius { get; private set; }
}

public static class Planets
{
    public static double GetSurfaceGravity(this Planet p)
    {
        PlanetAttr attr = GetAttr(p);
        return G * attr.Mass / (attr.Radius * attr.Radius);
    }

    public static double GetSurfaceWeight(this Planet p, double otherMass)
    {
        return otherMass * p.GetSurfaceGravity();
    }

    public const double G = 6.67300E-11;

    private static PlanetAttr GetAttr(Planet p)
    {
        return (PlanetAttr)Attribute.GetCustomAttribute(ForValue(p), typeof(PlanetAttr));
    }

    private static MemberInfo ForValue(Planet p)
    {
        return typeof(Planet).GetField(Enum.GetName(typeof(Planet), p));
    }

}

public enum Planet
{
    [PlanetAttr(3.303e+23, 2.4397e6)]  MERCURY,
    [PlanetAttr(4.869e+24, 6.0518e6)]  VENUS,
    [PlanetAttr(5.976e+24, 6.37814e6)] EARTH,
    [PlanetAttr(6.421e+23, 3.3972e6)]  MARS,
    [PlanetAttr(1.9e+27,   7.1492e7)]  JUPITER,
    [PlanetAttr(5.688e+26, 6.0268e7)]  SATURN,
    [PlanetAttr(8.686e+25, 2.5559e7)]  URANUS,
    [PlanetAttr(1.024e+26, 2.4746e7)]  NEPTUNE,
    [PlanetAttr(1.27e+22,  1.137e6)]   PLUTO
}
share|improve this question
    
I'm curious as to why you decided to use meta annotations to represent this data? Seems like an odd choice - attributes are designed to store meta data (data about data), but in this case you're storing properties of an entity - which is what object-orientated programming is all about! The fact that these extra properties are required means that an enum is not the appropriate data type to be using.. –  MattDavey Oct 14 '11 at 9:07
    
@MattDavey fair enough, its a contrived example and a dictionary or array would probably be a better way to link the enum to the object. The enum is still appropriate though - it represents the name of the planet, not the properties of the planet (which are in a separate class.) –  finnw Oct 14 '11 at 9:19
    
@MattDavey: 'meta' is a matter of perspective. One man's data can be another's metadata, and vice versa. You may consider database table definitions to be metadata for an LOB application, but it's data for an ORM designer. I draw the line in a pretty simple way - I consider any data written in the source code to be metadata (excluding sample data). The planets enum is not a shining example for the use of metadata, but continuing with that – in a program that manages servers by naming them after planets, any additional information about the actual astronomical bodies is metadata. –  Allon Guralnek Oct 21 '11 at 17:22
1  
@finnw: A dictionary would indeed be an appropriate run-time container for the metadata, but not a compile-time container. The dictionary can be constructed and populated from the metadata contained within the attributes either when the program starts up or when needed (lazy-loaded). –  Allon Guralnek Oct 21 '11 at 17:25

1 Answer 1

up vote 16 down vote accepted

Make it general purpose using generics.

Though I'd strongly suggest you rename your attribute to follow .NET guidelines. It should always be in the form AttributeNameAttribute. Enum values should be in CamelCase. And it is common to name classes containing extension methods to be named the same as the class it is extending followed by Extensions.

Otherwise, that's pretty much how you'd get custom attributes of a field.

public static class EnumExtensions
{
    public static TAttribute GetAttribute<TAttribute>(this Enum value)
        where TAttribute : Attribute
    {
        var type = value.GetType();
        var name = Enum.GetName(type, value);
        return type.GetField(name) // I prefer to get attributes this way
            .GetCustomAttributes(false)
            .OfType<TAttribute>()
            .SingleOrDefault();
    }
}

public class PlanetInfoAttribute : Attribute
{
    internal PlanetInfoAttribute(double mass, double radius)
    {
        this.Mass = mass;
        this.Radius = radius;
    }
    public double Mass { get; private set; }
    public double Radius { get; private set; }
}


public enum Planet
{
    [PlanetInfo(3.303e+23, 2.43970e6)]  Mecury,
    [PlanetInfo(4.869e+24, 6.05180e6)]  Venus,
    [PlanetInfo(5.976e+24, 6.37814e6)]  Earth,
    [PlanetInfo(6.421e+23, 3.39720e6)]  Mars,
    [PlanetInfo(1.900e+27, 7.14920e7)]  Jupiter,
    [PlanetInfo(5.688e+26, 6.02680e7)]  Saturn,
    [PlanetInfo(8.686e+25, 2.55590e7)]  Uranus,
    [PlanetInfo(1.024e+26, 2.47460e7)]  Neptune,
    [PlanetInfo(1.270e+22, 1.13700e6)]  Pluto,
}

public static class PlanetExtensions
{
    public static double GetSurfaceGravity(this Planet p)
    {
        var attr = p.GetAttribute<PlanetInfoAttribute>();
        return G * attr.Mass / (attr.Radius * attr.Radius);
    }

    public static double GetSurfaceWeight(this Planet p, double otherMass)
    {
        return otherMass * p.GetSurfaceGravity();
    }

    public const double G = 6.67300E-11;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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