views:

382

answers:

4

The code below is looping through a dictionary of strings and IMyCompanySettings looking for values that implement IMyCompanyProductSetting. Clearly, trying to cast and raising an exception is a very expensive way to do this.

    public static List<IMyCompanyProductSetting> GetProductSettings(ConfigurationManager cfm)
    {
        List<IMyCompanyProductSetting> ret = new List<IMyCompanyProductSetting>();
        foreach(IMyCompanySetting setting in cfm.Values)
        {
            try
            {
                IMyCompanyProductSetting prod = (IMyCompanyProductSetting)setting;
                ret.Add(prod);

            }
            catch
            {
              // Do nothing.
            }
        }
        return ret;
    }

What's a better way to do this?

+8  A: 

Casting 101 [general info on casting stuff]:

Use [object] is [interface/class] expression:

if (setting is IMyCompanyProductSetting) {
  ...
}

Alternatively you can use the as keyword which tries to cast the object and if it fails, instead of throwing exception, it'll return null. Note that the target type must be a reference type in the as keyword:

var prod = setting as IMyCompanyProductSetting; 

if (prod != null) {
   ...
}

You should always use the above code instead of the equivalent exception handling.

Filtering an IEnumerable by type (LINQy):

As Jon Skeet pointed out, you should use OfType extension method to filter a sequence easily (assuming you got LINQ):

var filteredSequence = sequence.OfType<TargetType>();

Casting an IEnumerable to type (LINQy):

If you want to try casting each element to the target type (as opposed to filtering by type), you can use the Cast extension method:

var castedSequence = sequence.Cast<TargetType>();
Mehrdad Afshari
That's not the easiest way to implement the method though, if you're using .NET 3.5... see my answer :)
Jon Skeet
Two up-votes and an accepted. That's 15 points a word! (Fortunately, it was just the memory jar I needed.)
Jekke
Jon Skeet, sure. However, as I saw the exception handling in the sample code, I thought it's more important to get rid of that fundamentally.
Mehrdad Afshari
Oh it's worth expanding on that too, sure. I just like to show the way to avoid having to write any real code in the first place.
Jon Skeet
+6  A: 

The "hard" way (pre-LINQ) is to use "as". This is more efficient than using "is" and then casting each time (as both the "is" and the cast require execution-time checks):

IMyCompanyProductSetting prod = setting as IMyCompanyProductSetting;
if (prod != null)
{
    ret.Add(prod);
}

See another question for when to use "as" and when to use a cast.

If you're using .NET 3.5, however, it's really easy:

return cfm.Values.OfType<IMyCompanyProductSetting>().ToList();

Very easy :)

Jon Skeet
+2  A: 

Mehrdad has the answer. I'll only add that you should never use that "try / catch everything" trick. At best in that case, you're trying to catch an InvalidCastException. You wouldn't want to ignore some other exception, possibly from the execution of the method you're trying to call.

John Saunders
+1  A: 

You should use the 'Is' statement instead for more concise and less error prone code. See the example below.

 if (setting Is IMyCompanyProductSetting)
  ret.add((IMyCompanyProductSetting)setting);
James