Make removing a formatter a 1-liner

[rynowak]

See code sample here: #2944 (comment)

When we were using descriptors for formatters we had a 1-liner extension method for removing a formatter by-type from the collection. We should add this back by defining custom collection types for input formatters and output formatters.

[Krusen]

Hi, I'm new to contributing to other projects but would like to give this a shot.

Were you thinking something like this?

namespace Microsoft.AspNet.Mvc.Core.Formatters
{
    public class FormatterCollection<TFormatter> : Collection<TFormatter>
    {
        /// <summary>
        /// Removes all formatters of the specified type.
        /// </summary>
        /// <typeparam name="T">The type to remove.</typeparam>
        public void RemoveType<T>() where T : TFormatter
        {
            var formatters = this.OfType<T>();
            foreach (var formatter in formatters)
            {
                Remove(formatter);
            }
        }
    }
}

public MvcOptions()
{
    // ...
    InputFormatters = new FormatterCollection<IInputFormatter>();
    OutputFormatters = new FormatterCollection<IOutputFormatter>();
    // ...
}

public FormatterCollection<IInputFormatter> InputFormatters { get; }
public FormatterCollection<IOutputFormatter> OutputFormatters { get; }

// ...

Should the IList<IInputFormatter>/IList<IOutputFormatter> properties in Microsoft.AspNet.Mvc.Filters.ResourceExecutingContext and Microsoft.AspNet.Mvc.Filters.ActionBindingContext be changed as well?

[rynowak]

@Krusen - yes, that's pretty much exactly what I had in mind.

/cc @Eilon in case he has feedback on the approach. This also follows the design of some other places where we were getting rid of extension methods that you need to use from options/startup code.

[Eilon]

I'm cool with this. BTW I'm not sure the proposed code will necessarily work: you might have to force .ToList() after calling .OfType() so that all the LINQ evaluation will be complete before modifying the collection.

[Krusen]

Great. I'll go ahead and make a pull request later today.

@Eilon You are right, it needs to be changed.

[davidfowl]

Avoid using LINQ if you can. If you use a List<T> you can use RemoveAll

[rynowak]

Avoid using LINQ if you can. If you use a List you can use RemoveAll

Why does this matter? This is about making startup code more friendly.

[rynowak]

ec18b35