What’s Wrong With This Code (#25)

The goal: create an extension method that will make it easy to create FormCollection objects. The method is a helper for unit testing ASP.NET MVC code.

public static FormCollection ToFormCollection(this object data)
{
    var namesAndValues =
        data.GetType()
            .GetProperties()
            .WhereValueIsNotDefaultValue(data)
            .ToNameValueCollection(data);
    
    return new FormCollection(namesAndValues);
}

The extension method itself relies on a couple private extension methods:

static IEnumerable<PropertyInfo> WhereValueIsNotDefaultValue(
    this IEnumerable<PropertyInfo> properties, object source)
{
    foreach(var p in properties)
    {
        var value = p.GetValue(source, null);
        if(value != (p.PropertyType.IsValueType ? 
                     Activator.CreateInstance(p.PropertyType)
                     : null))
        {
            yield return p;
        }
    }
}

static NameValueCollection ToNameValueCollection(
    this IEnumerable<PropertyInfo> properties, object source)
{
    return properties.Aggregate(
            new NameValueCollection(),
            (nvc,pi) =>
                {
                    var value = pi.GetValue(source, null);
                    nvc.Add(pi.Name, value.ToString());
                    return nvc;
                });
}

The code mostly works. This test passes:

public void should_copy_property_values()
{
    var expectedName = "Scott";
    var expectedHireDate = new DateTime(2010, 1, 1);

    var data = new Employee
    {
        Name = expectedName,
        HireDate = expectedHireDate
    }.ToFormCollection();

    Assert.IsTrue(data["Name"] == expectedName);
    Assert.IsTrue(data["HireDate"] == expectedHireDate.ToString());
}

However, the helper should not put entries in the FormCollection for properties holding a default value, and this test fails:

public void should_not_copy_default_values()
{
    var data = new Employee
    {
        Name = "",
        HireDate = DateTime.Now
    }.ToFormCollection();

    Assert.IsNull(data["Tags"]);
    Assert.IsNull(data["Id"]);
}

Employee is defined as:

public class Employee
{
    public int Id { get; set; }
    public string Name { get; set; }
    public DateTime HireDate { get; set; }
    public string Tags { get; set; }
}

Hint: the first assert passes – it’s the assert on data[“Id”] that fails.

Print | posted @ Monday, May 03, 2010 9:12 AM

Comments on this entry:

Gravatar # re: What’s Wrong With This Code (#25)
by Paul Cox at 5/3/2010 9:33 AM

The value variable in WhereValueIsNotDefaultValue is boxed and therefore compared by reference rather than by value?

It might be easier to read if implicit typing was not used for method calls. I've tried to only use var for object construction and where unavoidable for anonymous types.
  
Gravatar # re: What’s Wrong With This Code (#25)
by Scott at 5/3/2010 9:52 AM

@Paul - that's right. Because it's using != and testing object references.
  
Gravatar # re: What’s Wrong With This Code (#25)
by amir katz at 7/19/2010 11:54 AM

Can you use it with Equals?
That should compare value.

Thanks,
Amir
  
Comments have been closed on this topic.
Scott Allen
Posts - 869
Comments - 4493
Stories - 14