object initialization can be simplified

Solution 1:

While all the previous suggestions are also good, I would add a third way. Turn off those warnings and ignore them. While I appreciate Microsoft's attempts to get everyone coding efficiently and neatly, this is not a good suggestion in my opinion and it actually produces difficult to read and edit code.

Firstly, this essentially turns object initialisation into a single line of code, and any errors are reported as such. If you had 20 bits of data being loaded into an object you would be presented with an error on the first line and not told which property has errored. Debugging will not help as you are shown the whole block of code as the error.

Secondly, if you in future need to expand out the code and add additional code for a specific property you would now need to do this in seperate code. This adds to fragmentation and separates related bits of code(Maybe, it's debatable).

Both of these issues might seem like very minor things, but the warning suggests a fix that is also a very minor thing. For the sake of bracketing your initialisation you've made your code harder to debug and modify. This is a bad trade off in my opinion.

You can disable the warning by right-clicking the warning and selecting "suppress", or Go to Tools > Options > Text Editor > C# > Code Style > General > Prefer Object Initializer > and set the warning to None, or set the Preference to No. settings to change preference for object initializers

Solution 2:

1st

Before:

TreeNode node = new TreeNode(drive.Substring(0, 1), driveImage, driveImage);
node.Tag = drive;

After:

var node = new TreeNode(drive.Substring(0, 1), driveImage, driveImage) {
    Tag = drive
};

2nd

Before:

DirectoryInfo di = new DirectoryInfo(dir);
TreeNode node = new TreeNode(di.Name, 0, 1); //this line

After:

var node = new TreeNode((new DirectoryInfo(dir)).Name, 0, 1);

3rd

Before:

OleDbCommand select = new OleDbCommand();//this line
select.Connection = cnDTC;
select.CommandText = string.Format("SELECT MAX(VERSION_NO) AS MAX_VERSION FROM ({0})",
      strSQL2);

After:

var select = new OleDbCommand(
      String.Format("SELECT MAX(VERSION_NO) AS MAX_VERSION FROM ({0})", strSQL2), 
      cnDTC);

3rd (with string interpolation):

var select = new OleDbCommand($"SELECT MAX(VERSION_NO) AS MAX_VERSION FROM ({strSQL2})", 
      cnDTC);

BTW: whenever this kind of message appears, try putting the cursor on that line and hit Ctrl+. (or click the appearing lightbulb) - which opens up "quick-Fix / quick-refactor"

Further read on var (it really is not evil 😉) and some more documentation about Object and Collection Initializers

Solution 3:

I had a similar issue with this code:

        Customer oCust = new Customer();
        oCust.Address = txtAddress.Text;
        oCust.City = txtCity.Text;
        oCust.State = txtState.Text;

And solved it with this code:

        Customer oCust = new Customer()
        {
           Address = txtAddress.Text,
           City = txtCity.Text,
           State = txtState.Text
        };

Sooo...to turn off the warning message (IDE0017)(in VS 2017/2019):
Click the Tools Tab. Then go down to Options...
Then | TextEditor | C# | CodeStyle | General |
Under Expressoin preferences change Prefer Object Initializer to No.

Alternatively you could leave the Preference as Yes and change the Severity from Warning to Suggestion.
Now it will just show as a message in the Error List.

Solution 4:

I like @tonyenkiducx answer, but I feel as if there are some bigger-picture ideas that should be discussed.

From my experience, the refactor suggestions provided by Visual Studio are not helpful. I think the bigger question to consider is whether or not the code design is correct. In Object-Oriented Propgramming, setting properties one after another may violate encapsulation. The idea is that the object should always be in a valid state after any member is accessed/called until the moment the object is destroyed. In this case, the state should be valid after each property is set. Proper encapsulation will lead to an overall improved software application because you are increasing its cohesiveness.

The object initialization can be simplified messages may be useful in detecting points in your code where you can use a Creational Pattern, in the event encapsulation is violated:

  • Abstract factory pattern
  • Builder pattern
  • Factory method pattern
  • Prototype pattern

This allows us to address the concerns brought up by @tonyenkiducx:

Firstly, this essentially turns object initialisation into a single line of code, and any errors are reported as such. If you had 20 bits of data being loaded into an object you would be presented with an error on the first line and not told which property has errored. Debugging will not help as you are shown the whole block of code as the error.

Secondly, if you in future need to expand out the code and add additional code for a specific property you would now need to do this in seperate code. This adds to fragmentation and separates related bits of code(Maybe, it's debatable).

So, instead of inlining the instantiation at the point the object is consumed, which is what Visual Studio often suggests, I suggest you consider using a creational pattern. This may not remove the simplification message, but at this point, you've thoughtfully considered that message flag and can safely suppress it.