Use an Object Initializer? No thanks Resharper

I really like ReSharper in fact I’ll even go as far as to say I love it!

I also like object initializers in C#.

But as I’ll demonstrate in this post you can have too much of a good thing.

When object initializers go bad

The code below shows an example of mapping a model object from a data reader with and without using an object initializer.

using System;
using System.Data;

public class Mapper
{
    public static Model Map(IDataReader reader)
    {
        if (reader.Read())
        {
            Model model = new Model();
            model.FirstName = reader[0].ToString();
            model.LastName = reader[1].ToString();
            model.Age = Convert.ToInt32(reader[2].ToString());
            return model;
        }
        return null;
    }

    public static Model MapUsingObjectInitializer(IDataReader reader)
    {
        if (reader.Read())
        {
            return new Model
                              {
                                  FirstName = reader[0].ToString(),
                                  LastName = reader[1].ToString(),
                                  Age = Convert.ToInt32(reader[2].ToString())
                              };
        }
        return null;
    }
}

public class Model
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public int Age { get; set; }
}

If we run two unit tests to map the model from a data reader we will get two passing tests.

using System.Data;
using NUnit.Framework;

[TestFixture]
public class When_Mapping_Model_Using_DataReader
{
    [Test]
    public void Should_Be_Able_To_Map_Model()
    {
        Mapper.Map(GetDataReader());
    }

    [Test]
    public void Should_Be_Able_To_Map_Model_Using_Object_Initializer()
    {
        Mapper.MapUsingObjectInitializer(GetDataReader());
    }

    public static IDataReader GetDataReader()
    {
        DataTable table = new DataTable();
        DataRow row = table.NewRow();
        table.Columns.Add(new DataColumn("FirstName"));
        table.Columns.Add(new DataColumn("LastName"));
        table.Columns.Add(new DataColumn("Age"));
        row["FirstName"] = "Bob";
        row["LastName"] = "Smith";
        row["Age"] = "55";
        table.Rows.Add(row);
        return new DataTableReader(table);
    }
}

Object initializers passing tests

Now if we modified the GetDataReader method to return a string value for age

public static IDataReader GetDataReader()
{
    DataTable table = new DataTable();
    DataRow row = table.NewRow();
    table.Columns.Add(new DataColumn("FirstName"));
    table.Columns.Add(new DataColumn("LastName"));
    table.Columns.Add(new DataColumn("Age"));
    row["FirstName"] = "Bob";
    row["LastName"] = "Smith";
    row["Age"] = "FiftyFive";
    table.Rows.Add(row);
    return new DataTableReader(table);
}

We get a failing test when mapping without an object initializer as shown below

As you can see from the output message above we can easily find the line of code (line 13) that caused the exception.

When a test fails for the code using an object initializer

the line of code that caused the exception (line 23) is the same one in which the object was created.

The fact is the more properties you initialize using an object initializer the harder it will be to find the one that causes an exception.

What’s ReSharper got to do with this?

In a word temptation. It makes it so easy to convert your code to use one.


That’s not ReSharpers fault

I agree. Ultimately it’s a developers choice if they want to use the refactoring that tools like ReSharper suggest, after all just because you can doesn’t mean you should.

However it could give you an option not to convert to object initializer if more than say ten properties were going to be initialized.

Or detect if the object initializer wasn’t a straight forward mapping and then not suggest using one like in this example.

What ReSharper is missing

What would be really useful if you could convert code that was using an object initializer to the classic way of setting properties one line at a time.

C’mon JetBrains how about it?

It turns out you can do this in ReSharper check out this post on using assignment statements.

Comments

  • http://www.facebook.com/jeroenpot Jeroen Pot

    Oh, that would be a sweet new feature for R#. I’m always trying hard to forbid object initializers that use more then 2 lines.

  • http://www.facebook.com/andrewdj Andrew Johns

    I usually use the feature to identify where I can add a constructor to the class in question, which gives you the best of both worlds, I think: tidier but still allows you to debug the code pretty easily.

    Obviously this isn’t relevant when working with a class that you don’t have internal access to modify though.

  • Jenya Legkiy

    What ReSharper is missing…
    Actually there is context action that converts initializer to assignments.
    Just position cursor at ‘ = ‘ sign or initializer braces

  • http://sm-art.biz Artëm Smirnov

    The problem is not with the object initializers. Or Resharper. The problem is with putting too much action into a single line of code. You can do it in a lean way: use object initializers until you get a problem, then put everything that could cause an exception into a single line.

  • Anonymous

    Yep as I said in the post it’s down to developers to keep their code clean