1
\$\begingroup\$

Here's my custom formatter which I want to use for "application/x.product" content type:

public class ProductFormatter : MediaTypeFormatter
    {
        public ProductFormatter()
        {
            SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/x.product"));
        }
        public override bool CanReadType(Type type)
        {
            return false;
        }
        public override bool CanWriteType(Type type)
        {
            var result = (type == typeof(Product) || type == typeof(IEnumerable<Product>));
            return result;
        }

        public override async Task WriteToStreamAsync(Type type, object value, Stream writeStream, HttpContent content, TransportContext transportContext)
        {
            List<string> productStrings = new List<string>();
            IEnumerable<Product> products = value is Product ? new Product[] { (Product)value } : (IEnumerable<Product>)value;
            foreach (Product product in products)
            {
                productStrings.Add(string.Format("{0},{1},{2}",
                product.ProductID, product.Name, product.Price));
            }
            StreamWriter writer = new StreamWriter(writeStream);
            await writer.WriteAsync(string.Join(",", productStrings));
            writer.Flush();
        }
    }

Registered as:

config.Formatters.Add(new ProductFormatter());

I also have a custom negotiator (also registered):

public class CustomNegotiator : DefaultContentNegotiator
    {
        public override ContentNegotiationResult Negotiate(Type type,
        HttpRequestMessage request, IEnumerable<MediaTypeFormatter> formatters)
        {
            // look for an appropriate formatter
            MediaTypeFormatter formatter = null;
            foreach(var f in formatters)
            {
                var mediaType = f.SupportedMediaTypes.Where(mt => mt.MediaType == "application/x.product").SingleOrDefault();
                if (mediaType != null)
                {
                    formatter = f;
                    break;
                }
            }
            if (request.Headers.UserAgent.Where(x => x.Product != null
            && x.Product.Name.ToLower().Equals("chrome")).Count() > 0 && formatter != null)
            {
                // if client is chrome then return superproduct and use custom formatter for it
                return new ContentNegotiationResult(formatter,
                new MediaTypeHeaderValue("application/x.product")
                );
            }
            else
            {
                return base.Negotiate(type, request, formatters);
            }
        }
    }

I also want to use this with application/x.product when a client is Google Chrome.

Is my way of searching an appropriate formatter okay, or is there a better way?

\$\endgroup\$
1
\$\begingroup\$

ProductFormatter

The WriteToStreamAsync() method could be improved by using the using keyword with the StreamWriter. By using using the flushing and disposing of the stream is done automatically.

Having a string.Format() like it is used together with the string.Join() is suboptimal. You should better add the product details to the List<string> and let the string.Join() to the work.

Implementing this points will lead to

public override async Task WriteToStreamAsync(Type type, object value, Stream writeStream, HttpContent content, TransportContext transportContext)
{
    List<string> productStrings = new List<string>();
    IEnumerable<Product> products = value is Product ? new Product[] { (Product)value } : (IEnumerable<Product>)value;
    foreach (Product product in products)
    {
        productStrings.Add(product.ProductID);
        productStrings.Add(product.Name);
        productStrings.Add(product.Price);
    } 

    using (StreamWriter writer = new StreamWriter(writeStream))
    {
        await writer.WriteAsync(string.Join(",", productStrings));
    }
}  

You should also take into account that object value could be null. Assuming that it is an IEnumerable<Product> if it isn't a Product type is suboptimal.


CustomNegotiator

The retrieving of the formatter should be extracted to a separate method otherwise the Negotiate() method would do to many things and would violate the single responsibility principle.

The way how you are doing the search is ok, but could be improved by extracting the "application/x.product" to a class constant.

This if

if (request.Headers.UserAgent.Where(x => x.Product != null
&& x.Product.Name.ToLower().Equals("chrome")).Count() > 0 && formatter != null)  

should be improved by first checking if formatter != null and to use Any() instead of Count() > 0. By using Count() > 0 all of the items will be iterated over and counted. Using Any() will iterate only until any item is found for which the condition is true. Instead of x.Product.Name.ToLower().Equals("chrome") you should use the overloaded Equals() method which takes a StringComparision enum as second parameter. This buyes you two things. The first is the call to ToLower() isn't needed anymore and the second is that your code will pass the turkey test.

Implementing this points will lead to

private const string mediaType = "application/x.product";
private MediaTypeFormatter FindFormatter(IEnumerable<MediaTypeFormatter> formatters)
{

    foreach (var formatter in formatters)
    {
        var foundMediaType = formatter.SupportedMediaTypes.Where(mt => mt.MediaType == mediaType).SingleOrDefault();
        if (foundMediaType != null)
        {
            return formatter;
        }
    }

    return null;
}  

public class CustomNegotiator : DefaultContentNegotiator
{
    public override ContentNegotiationResult Negotiate(Type type,
                HttpRequestMessage request, IEnumerable<MediaTypeFormatter> formatters)
    {

        var formatter = FindFormatter(formatters);

        if (formatter != null && request.Headers.UserAgent.Any(x => x.Product != null
                                 && x.Product.Name.Equals("chrome", StringComparison.OrdinalIgnoreCase)))
        {
            // if client is chrome then return superproduct and use custom formatter for it
            return new ContentNegotiationResult(formatter,
                            new MediaTypeHeaderValue(mediaType));
        }
        else
        {
            return base.Negotiate(type, request, formatters);
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Many thanks for reviewing it thoroughly!!! \$\endgroup\$ – amplifier Nov 2 '15 at 9:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.