Find attribute on class property

Since reading Robert Martin's book "Clean Code" I have been inspired to revisit and refactor some of my code to break it down into succinct methods and small specialised classes.

Given the need to retrieve the NameAttribute off the Name2 property of the TestObject class below...

public class TestObject
    {
        public string Name1 { get; set; }
        [Name("Address")]
        public string Name2 { get; set; }
    }

I have built a class called PropertyNameAttributeFinder to acheive this.

```
public class PropertyNameAttributeFinder
{
#region Fields

private readonly PropertyInfo _property;
private NameAttribute _attributeFound;

#endregion

#region Construtors

///
/// Initializes a new instance of the class.
///
/// The property.
/// property
public PropertyNameAttributeFinder(PropertyInfo property)
{
if (property == null) throw new ArgumentNullException("property");

_property = property;
}

#endregion

#region Public Members

///
/// Checks for attribute.
///
/// The current instance for fluid API
public PropertyNameAttributeFinder CheckForAttribute()
{
SetAttributeIfExists();
return this;
}

///
/// Gets a value indicating whether this instance has found an attribute.
///
///
/// true if this instance has found an attribute; otherwise, false.
///
public bool HasFoundAttribute
{
get { return _attributeFound != null; }
}

///
/// Gets the AttributeFound that was found.
///
///
/// The AttributeFound.
///
public NameAttribute AttributeFound
{
get
{
if (!HasFoundAttribute)
{
throw new InvalidOperationException("No attribute type was found so cannot be returned. Hint: Use HasFoundAttribute first.");
}
return _attributeFound;
}
}

#endregion

I think that Heslacher is right and you don't strictly need this class, but as you've written it I'd like to point out a couple of things:

• Many people, myself included, don't like to use regions. They just add noise - especially on such a small class

• SetAttributeIfExists is one step too far IMO. The method name isn't accurate either, you're setting the value if it exists or not.

• The exception message is really odd if you specify a property without the attribute:

[TestMethod]
public void AttributeFound_WhenCalledAfterCheckAttributeAndPropertyDoesntHaveAtrribute_ReturnsNull()
{
    // ARRANGE
    Type testType = typeof(TestObject);
    PropertyInfo property = testType.GetProperty("Name1");

    // ACT
    NameAttribute actual = new PropertyNameAttributeFinder(property)
        .CheckForAttribute()
        .AttributeFound;

    // ASSERT
    Assert.IsNull(actual);
}

I would get an exception saying this:


No attribute type was found so cannot be returned. Hint: Use HasFoundAttribute first.

That's not a good hint - I've already called HasFoundAttribute. The message could also be more specific as you didn't find a NameAttribute not any old attribute type. The point is you've extended 1 line of code to an entire class for no benefit.

StackExchange Code Review Q#114624, answer score: 3