Should this be a warning, at least?

[Guest]

The C# compiler gives warnings about unused variables. I thin it needs a
warning about an unassigned reference. For instance, this compiles without
any warnings/errors, but of course blows up when run:

SqlConnection cnn = null;
try
{
new SqlConnection(myConnectionString);
cnn.Open()


Comments?

Rich Sienkiewicz
Nice Systems, Inc.

 

[Matt]

The C# compiler gives warnings about unused variables. I thin it needs a
warning about an unassigned reference. For instance, this compiles without
any warnings/errors, but of course blows up when run:

SqlConnection cnn = null;
try
{
new SqlConnection(myConnectionString);
cnn.Open()


Comments?

I'd say no. Its too hard to figure out if cnn could have been set as a
side-effect
of some other action. Yes, this particular one would be easy to spot,
but cnn
IS assigned to something (null). If you just wrote:

SqlConnection cnn;

cnn.Open();

I'd expect an error, or a warning at the very least.

Matt

 

[Stoitcho Goutsev \(100\)]

Rich,

You initialize cnn by setting *null*. If you don't do that the compiler will
complain that you are trying to use unitialized variable.

 

[Nicholas Paldino [.NET/C# MVP]]

Rich,

I agree, for this situation, it would be nice, but it would be very
limited, limited only by variables declared on the stack. If they were
parameters or fields, it would require one to do a much more intensive
static analysis.

Hope this helps.

 

[Guest]

I'd wonder if Spec# would catch this. Too bad it's still in R&D.

Anyone know if I can safely run Spec# on the same production dev box and not
mess up my reliable C# compiler? I'd like to play with this...

 

[Bruce Wood]

Just to clarify, are you saying that the compiler should issue a
warning about this line:

new SqlConnection(myConnectionString);

because the reference returned from "new" wasn't assigned to anything?

Or are you saying that the compiler should issue a warning about this
line:

cnn.Open();

because cnn will be null at run time?

The first seems reasonable to me, although it seems an unlikely mistake
to make. True, the compiler doesn't _really_ know that doing the "new"
doesn't have some useful side-effect that in fact you don't want to
assign the result to anything, but such a warning would be trivially
eliminated by just doing this:

SqlConnection dummy = new SqlConnection(myConnectionString);

so it's not a bad idea, although I'm not sure how useful it would be
because I've never seen that mistake, myself.

The second wouldn't fly because assigning "null" to cnn means that it
has been assigned. The following, on the other hand, already generates
a warning (I believe):

SqlConnection cnn;
try
{
new SqlConnection(myConnectionString);
cnn.Open();
}

because the compiler knows that "cnn" hasn't been given any value at
all, not even null.

 

[Guest]

To clarify, I would like the compiler to give a warning about the result of
the new statement not being assigned to anything. Like I said, the compiler
warns of non used variables, of non-assigned variables, so it could issue a
warning about a missing assignment. What use could this new statement, or any
other new, be if it's not assigned to something?

BTW, the error was made during a hasty cut and paste.

Thanks everyone

Rich

 

[Jon Skeet [C# MVP]]

To clarify, I would like the compiler to give a warning about the result of
the new statement not being assigned to anything. Like I said, the compiler
warns of non used variables, of non-assigned variables, so it could issue a
warning about a missing assignment. What use could this new statement, or any
other new, be if it's not assigned to something?

Constructing a new instance might do any number of things in the
background, such as registering the new instance in a pool. It's
unlikely to be a good thing, but I don't think the language ought to
complain. Would you want the language to complain about calling methods
but not using the return value? It's much the same issue.

Jon

 

[Bruce Wood]

With all due respect, Jon, it's not all that much the same issue.

Calling a method but not using its return value (or using an operator
but not storing the result) is a common technique for causing some
side-effect. (Viz: the oh-so-common "i++;")

However, "new"ing an object but not storing it anywhere is only very
rarely a useful thing to do, and, in those very few cases in which it
is useful, it's easy enough to assign the result to a "dummy" variable
in order to have the compiler shut up.

That said, I would also say that this is such an uncommon mistake to
make (newing something but not assigning it to a variable) that it's
unlikely to make it anywhere near the top of the list of compiler mods.

 

[Stoitcho Goutsev \(100\)]

Bruce,

I agree with Jon,

If the compiler force you to create dummy variables then one is going to
complain about this. The compiler should do anything possible to help you
spot problems without being intrusive forcing you to write obviously
redundant code, otherwise it is going to be more like fighting with the
compiler than writing programs.

 

[Ignacio Machin \( .NET/ C# MVP \)]

Hi,

With all due respect, Jon, it's not all that much the same issue.

Calling a method but not using its return value (or using an operator
but not storing the result) is a common technique for causing some
side-effect. (Viz: the oh-so-common "i++;")

In the framework itself you have a lot of methods like this, take for
example
SqlCommand.Parameters.Add ( ... ) more often you do not use the returned
Parameter ( at least directly )
Another good example is DataTable.Column.Add , unless you need to especify
some extra features ( PK, autoincrement) you just ignore the returned object

However, "new"ing an object but not storing it anywhere is only very
rarely a useful thing to do, and, in those very few cases in which it
is useful, it's easy enough to assign the result to a "dummy" variable
in order to have the compiler shut up.

I use construction like this very often:

new A_Windows_Form().Show();

I dont need to store a reference to it, so I don't

I agree with Jon, this should not be even a warning