Lambda Expression - is it evaluating correctly?

Question

I have the following piece of code which doesn't seem to be acting right. There is a property which has one attribute which is not of type FieldMapAttribute, but still it goes into the if condition where I am checking the count matching that type of attribute.

foreach (PropertyInfo _property in _properties)
{
    var attributes = _property.GetCustomAttributes(false);
    if (attributes.Select(a => a.GetType() == typeof(FieldMapAttribute)).Count() > 0)
    {
        colname = (attributes.Select(a => a.GetType() == typeof(FieldMapAttribute)).Cast<FieldMapAttribute>().First()).DbColumnName;
    }
}

Can someone help me to understand what is going on here?

Adi Lester · Answer 1 · 2012-08-25T19:55:31.007

Assuming what you're trying to do is check whether a FieldMapAttribute attribute exists on the property, you should use

var attributes = _property.GetCustomAttributes(typeof(FieldMapAttribute), false);
if (attributes.Length > 0)
{
    ...
}

Another option is to use

if (attributes.OfType<FieldMapAttribute>().Any())

You should note that the way you use Select is incorrect. Select is used to project elements into a new form. Your Select statement returns a list of bools - one for each attribute the property has (any attribute, not just of type FieldMapAttribute). This means that if your property looked like this

[SomeAttribute]
[SomeOtherAttribute]
[FieldMapAttribute]
public string MyProp { get; set; }

Then your select statement would yield the following result

false
false
true

As you can see, calling Count on this result set will always return the number of custom attributes set on the property (again, any attribute).

Hope this helps.

Not my downvote, but PropertyInfo.IsDefined is a simpler approach for the test. — Jon Skeet, Aug 25 '12 at 19:53
Both the answers are very helpful and I really appreciate Jon & Adi both. Thanks guys I seem to be running smoothly again. — sherebiah.tishbi, Aug 25 '12 at 20:10

Jon Skeet · Answer 2 · 2012-08-25T20:18:23.223

2

Regardless of exactly what's going on in your current code, it looks to me like it could be written much more simply:

foreach (PropertyInfo property in properties)
{
    if (property.IsDefined(typeof(FieldMapAttribute), false))
    {
        colName = property.GetCustomAttributes(typeof(FieldMapAttribute), false)
                          .Cast<FieldMapAttribute>()
                          .First()
                          .DbColumnName;            
    }
}

(Note that that will end up with the last property defining the attribute as the one which specifies the column name. Is that what you want?)

Or even using LINQ for the whole thing:

var attrType = typeof(FieldMapAttribute);
var attr = properties.SelectMany(p => p.GetCustomAttributes(attrType), false)
                     .Cast<FieldMapAttribute>()
                     .FirstOrDefault();
if (attr != null)
{
    colName = attr.DbColumnName;
}

edited Aug 25 '12 at 20:18

answered Aug 25 '12 at 19:52

Jon Skeet

1,421,763
867
9,128
9,194

I like this approach, but wouldn't this cause looking up the property's attributes twice instead of once in cases where `FieldMapAttribute` was defined? – Adi Lester Aug 25 '12 at 19:59
@AdiLester: Potentially (although not in the second option). Unless I had any reason to think that was actually a bottleneck though, I wouldn't worry about it. – Jon Skeet Aug 25 '12 at 20:01
Thanks Jon...I appreciate your reply and help. :) – sherebiah.tishbi Aug 25 '12 at 20:11
Jon I had to make small change in your suggestion to make it work. colname = (_property.GetCustomAttributes(typeof(FieldMapAttribute), false).Cast().First()).DbColumnName; – sherebiah.tishbi Aug 25 '12 at 20:16

Lambda Expression - is it evaluating correctly?

2 Answers2