Creating instance of all classes that have certain attribut value

Question

I have a simple custom attribute MessageDeliveryAttribute that has one string filed. Also I have several classes that mark with this attribute like:

[MessageDeliveryAttribute("MessegaToDb")]

In the app.config are settings like:

  <appSettings>
    <add key="MessageToConsole" value="true"/>
    <add key="MessageToDb" value="true"/>
    <add key="MessageToService" value="false"/>
    <add key="MessageToFile" value="false"/>
  </appSettings>

I'm trying to implement class that will read all keys with value "true" from app.config, find all classes that marked with attribute

[MessageDeliveryAttribute("value of key from app.config where value = true")]

and then create a list with instance of these classes.
Here are working code, but I need your suggestions how to improve that?

public static List<IMessageDelivery> GetMessagesDelivery()
{
    var instances = new List<IMessageDelivery>();

    // list of settings from app.config where value is true
    List<string> settingValues = ConfigurationManager.AppSettings.AllKeys
        .Where(key => ConfigurationManager.AppSettings[key].Equals("true"))
        .Select(value => value)
        .ToList();


    foreach (var types in GetTypesWithMessageDeliveryAttribute())
    {
        var attributeValue = GetAttributeValue(types);
        if (settingValues.IndexOf(attributeValue) >= 0)
        {
            instances.Add(Activator.CreateInstance(types) as IMessageDelivery); 
        }                 
    }

    return instances;
}
// Get all classes with my custom attribute
public static IEnumerable<Type> GetTypesWithMessageDeliveryAttribute()
{
    var assembly = Assembly.GetExecutingAssembly();
    return from type in assembly.GetTypes()
           where type.IsDefined(typeof(MessageDeliveryAttribute), false)
           select type;
}
// get value of attribute
public static string GetAttributeValue(Type t)
{
    var messageDeliveryAttribute = (MessageDeliveryAttribute)Attribute.GetCustomAttribute(t, typeof(MessageDeliveryAttribute));

    return messageDeliveryAttribute != null ? messageDeliveryAttribute.DeliveryPointName : string.Empty;
}

Answer 1

I'm not sure what feedback specifically you're looking for. There are minor formatting and naming questions, but only two things I would specifically draw attention to.

The first thing is that you're enumerating all the settings in the App.config, but you're only interested in settings that relate directly to message delivery. You should name-space your setting keys, and filter the settings before evaluating them.

<appSettings>
    <add key="MessageDelivery.MessageToConsole" value="true"/>
    <add key="MessageDelivery.MessageToDb" value="true"/>
    <add key="MessageDelivery.MessageToService" value="false"/>
    <add key="MessageDelivery.MessageToFile" value="false"/>
  </appSettings>

and

List<string> settingValues = ConfigurationManager.AppSettings.AllKeys
        .Where(key => ConfigurationManager.AppSettings[key].Equals("true"))
        .Where(key => key.StartsWith("MessageDelivery."))
        .Select(value => value.Replace("MessageDelivery.",""))
        .ToList();

---

Secondly, avoid using two different forms of Linq expression building:

List<string> settingValues = ConfigurationManager.AppSettings.AllKeys

        .Where(key => ConfigurationManager.AppSettings[key].Equals("true"))
        .Select(value => value)
        .ToList();

and

return from type in assembly.GetTypes()
           where type.IsDefined(typeof(MessageDeliveryAttribute), false)
           select type;