3
\$\begingroup\$

Im constructing metadata about a class. With reflection i iterate over the class properties, and those properties have attributes (name, validation etc). When iterating those attributes, i typecast to some specific attribute-types, and if successful store the relevant attribute information. Im looking to do this in a more elegant way since there is much code repetition at this point. In part the typecast checks (attibute as Attributetype), and also the check if Validate is null, and then new-ing it up is repetitive (Validate needs to be null if there are no validation attributes). I expect the number of attribute-types to grow

Ideal would be to define what attribute-types im looking for, and if found execute relevant code.

foreach (Attribute attribute in property.GetCustomAttributes(true))
{

    //Info attribute
    DisplayAttribute display = attribute as DisplayAttribute; 
    if (display != null)
    {
        Name =display.Name;
        Group = display.GroupName;
        Description = display.Description;
    }

    //Validation attributes
    MinLengthAttribute min = attribute as MinLengthAttribute;
    if (min != null)
    {
        if (Validate == null) Validate = new MetaValidate();
        Validate.MinLength = min.Length;
    }

    MaxLengthAttribute max = attribute as MaxLengthAttribute;
    if (max != null)
    {
        if (Validate == null) Validate = new MetaValidate();
        Validate.MaxLength = max.Length;
    }

    RegularExpressionAttribute regex = attribute as RegularExpressionAttribute;
    if (regex != null)
    {
        if (Validate == null) Validate = new MetaValidate();
        Validate.Regex = regex.Pattern;
        Validate.RegexError = regex.ErrorMessage;
    }
}
\$\endgroup\$
0

2 Answers 2

1
\$\begingroup\$

Seems like ive found a great solution, my entire loop can be rewritten as this:

InvokeOnAttr<DisplayAttribute>(property, a => {
    Name = a.Name;
    Group = a.GroupName;
    Description = a.Description;
});

InvokeOnAttr<MinLengthAttribute>(property, a => {
    Validate.MinLength = a.Length;
});

InvokeOnAttr<MaxLengthAttribute>(property, a => {
    Validate.MaxLength = a.Length;
});

InvokeOnAttr<RegularExpressionAttribute>(property, a => {
    Validate.Regex = a.Pattern;
    Validate.RegexError = a.ErrorMessage;
});

If this method also is availible in the class (it takes a property, finds the attribute (of type TAttr provided), and if found executes the passed in action on it:

public void InvokeOnAttr<TAttr>(PropertyInfo p, Action<TAttr> action) where TAttr : Attribute {
    TAttr attr = p.GetCustomAttribute<TAttr>();
    if (attr is ValidationAttribute)
    {
        if (Validate == null) Validate = new MetaValidate();
    }
    if (attr != null) { 
        action.Invoke(attr);
    }
}
\$\endgroup\$
0
\$\begingroup\$

Nothing wrong with your code.

If you want you can use linq if performance is not critical. PseudoCode:

var displayAttribute = type.GetProperties()
                           .SelectMany(p => p.GetCustomAttributes(true))
                           .OfType<DisplayAttribute>()
                           .SingleOrDefault();
if (displayAttribute != null)
{
    ...      
}

Nice and declarative, you see what the code does at a glance. And you can compose with Single, SingleOrDefault, First, FirstOrDefault etc. as you need.


For the validation thing a factory method cleans things up:

private static MetaValidate CreateMetaValidate(
    MinLengthAttribute minAttribute, 
    MaxLengthAttribute maxAttribute,
    RegularExpressionAttribute regexAttribute)
{
    var validate = new MetaValidate
    {
        Min = minAttribute?.Min ?? 0,
        Max = maxAttribute?.Max ?? 16,
        ...
    };

    return validate;
}

Again pseudocode unless it happens to compile.

Thought of one thing that might be a pretty big smell:

What is up with the property initializers? When the data is read from attributes it feels like it should be readonly. If there is not a reason that they are mutable you should make the properties readonly and use constructor.

\$\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.