object initialization can be simplified

Question

With my code I get 3 messages all saying object initialization can be simplified and in my ever growing thirst for knowledge (and my OCD) I would like to "fix" my code so that these messages dont appear. I know I can just set it so these messages dont appear but I will still have in my head that they are there in the background which doesnt sit right with me. If anyone can point out how to "simplify the initialisation" that would be great so I can improve my skills. If more code is required let me know and I can add it in.

1st:

TreeNode node = new TreeNode(drive.Substring(0, 1), driveImage, driveImage);//issue on this line
node.Tag = drive;

2nd:

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

I suspect with the treenodes its because I have given them the same name but I tried changing the name but it didn't make a difference.

3rd:

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

Answer 1

NEW with C# 9 : this "error"/warning message from the IDE (here Visual Studio) is just a reminder that you can make your code (a little bit) shorter, or less repetitive I would say.

Here, you just dont repeat "TreeNode" twice in the same line of code:

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

becomes

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

OR (as indicated above)

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

Shorter, and still clear.

Voila.

Answer 2

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.

Answer 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.

Answer 4

The compiler wants you to use the following syntax:

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

This is for your 3rd case.

Answer 5

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

Answer 6

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.