Try Catch Finally , pointers and Vb.net 2005 compiler warnings

[Bob]

Hi,
The compiler gives Warning 96 Variable 'cmdSource' is used before it has
been assigned a value. A null reference exception could result at runtime.

Dim cmdSource as SQlClient.SQLDataReader
Try
Set up the database read and do it.
Catch ex as system.exception
exception stuff here
Finally
cmdSource.close()
End try

Can't see a way out of this (Assuming a desire to explicity close in the
finally)
If you Dim inside the try statement you go out of scope in the finally.

Is there a tidy way to get rid of these warnings? My (was Vb.net 2003) Data
acccess class is now littered with them.
Thanks
Bob

 

[David Browne]

Hi,
The compiler gives Warning 96 Variable 'cmdSource' is used before it has
been assigned a value. A null reference exception could result at runtime.

Dim cmdSource as SQlClient.SQLDataReader
Try
Set up the database read and do it.
Catch ex as system.exception
exception stuff here
Finally
cmdSource.close()
End try

With Try/Catch there is no reason to enter the Try block until you have
sucessfully opened the DataReader. On an exception before you have
initialized the DataReader does not require you to Dispose it.

dim cmdSource as SqlDataReader = cmd.ExecuteReader()
Try
'whatever
Catch ex as Exception
cmdSource.Dispose
throw
end try

In VB 2005 you should replace all these with Using blocks:

Using cmdSource As SqlClient.SqlDataReader = cmd.ExecuteReader()
'whatever
End Using


David

 

[CT]

Bob,

This will do the trick,

Dim cmdSource as SQlClient.SQLDataReader = Nothing

Alternatively, you can switch off this checking for a project, by opening
the Property Pages, clicking Compile tab, and clicking None on the
Notification list for the "Use of variable prior to assignment " condition.

 

[Cor Ligthert [MVP]]

Bob,

Dim cmdSource as SQlClient.SQLDataReader
Try
Set up the database read and do it.

Something as

dim cmdSource As SQLClient.SQLDataReader = cmd.ExecuteReader()
I hope this helps,

Cor

 

[Cor Ligthert [MVP]]

Carsten,

Dim cmdSource as SQlClient.SQLDataReader = Nothing

This is not the trick, it *hides* only the warning with an extra instruction
that has to be done.

In my opinion is better to correct for what is warned.

Cor

 

[CT]

Nope, it doesn't hide the warning. The warning is because the variable is
used prior to assignment.

"= Nothing" is the assignment... I'm sure I know what you're getting at,
i.e. use the ExecuteReader method, but why not do that in the Try block,
which is probably what is being done, and in that case, my solution will
work.

 

[CT]

That will only work if the ExecuteReader shouldn't be wrapped in a Try
block. Obviosuly, you can declare cmdSource inside the the Try block as
well, but what if you need access to it outside the Try block? Then you need
to move the declaration at the top, using Nothing as an assignment to avoid
the warning...

 

[Herfried K. Wagner [MVP]]

The compiler gives Warning 96 Variable 'cmdSource' is used before it has
been assigned a value. A null reference exception could result at runtime.

Dim cmdSource as SQlClient.SQLDataReader
Try
Set up the database read and do it.
Catch ex as system.exception
exception stuff here
Finally

\\\
If cmdSource IsNot Nothing Then
///

cmdSource.close()

\\\
End If
///

End try

Can't see a way out of this (Assuming a desire to explicity close in the
finally)
If you Dim inside the try statement you go out of scope in the finally.

I currently do not have VB 2005 at hand, thus I am not sure if it will cause
the warning to disappear.

 

[Jay B. Harlow [MVP - Outlook]]

Bob,
In addition to the other comments, consider using a Using statement instead.

http://msdn2.microsoft.com/en-us/library/htd05whh(en-us,vs.80).aspx

Something like:

' VB 2005 syntax
Using cmdSource As SQLClient.SQLDataReader = cmd.ExecuteReader()
' process reader here...
End Using

As the Using statement does your Try/Finally with a check for not Nothing as
Herfried shows.

If you want to handle exceptions locally, then embed a Try/Catch.

' VB 2005 syntax
Using cmdSource As SQLClient.SQLDataReader = cmd.ExecuteReader()

| Try
' process reader here...
| Catch ex as system.exception
| exception stuff here
| End try
End Using

Normally I do not include the Try/Catch locally with the Using as above as I
use "global" Try/Catch blocks at a higher level then the read itself.

--
Hope this helps
Jay [MVP - Outlook]
..NET Application Architect, Enthusiast, & Evangelist
T.S. Bradley - http://www.tsbradley.net


| Hi,
| The compiler gives Warning 96 Variable 'cmdSource' is used before it has
| been assigned a value. A null reference exception could result at runtime.
|
| Dim cmdSource as SQlClient.SQLDataReader
| Try
| Set up the database read and do it.
| Catch ex as system.exception
| exception stuff here
| Finally
| cmdSource.close()
| End try
|
| Can't see a way out of this (Assuming a desire to explicity close in the
| finally)
| If you Dim inside the try statement you go out of scope in the finally.
|
| Is there a tidy way to get rid of these warnings? My (was Vb.net 2003)
Data
| acccess class is now littered with them.
| Thanks
| Bob
|
|

 

[Cor Ligthert [MVP]]

Jay,

This was I missing from Herfried and why I did not tell it.

Normally I do not include the Try/Catch locally with the Using as above as
I
use "global" Try/Catch blocks at a higher level then the read itself.

In my opinion is that a must with a datareader where a connection can give
trouble.



Cor

 

[Cor Ligthert [MVP]]

Doh,

I thought Herfried had showed the using block here. He did not here probably
I saw him do that somewhere else, because I thought I had seen him doing
that. (I had the idea to show a sample with using here, however my problem
with the using is that you miss than the catch)

And as you probably know am I not so much a favorite for a global catch what
is than in my opinion a solution.

Just a preference.

Cor

 

[Jay B. Harlow [MVP - Outlook]]

Cor,
| > Normally I do not include the Try/Catch locally with the Using as above
as
| In my opinion is that a must with a datareader where a connection can give
| trouble.

Why is the Catch "a must"?

I only include a Catch when there is something specific to do, such as
retrying the read. Logging the exception is not specific enough as the
global exception handlers (or a top level/main routine try/catch) can log
the error. However throwing a new exception with context information, such
as product id, is specific enough, as then the global exception handlers
would have more context sensitive information to display...


--
Hope this helps
Jay [MVP - Outlook]
..NET Application Architect, Enthusiast, & Evangelist
T.S. Bradley - http://www.tsbradley.net


| Jay,
|
| This was I missing from Herfried and why I did not tell it.
|
| > Normally I do not include the Try/Catch locally with the Using as above
as
| > I
| > use "global" Try/Catch blocks at a higher level then the read itself.
|
| In my opinion is that a must with a datareader where a connection can give
| trouble.
|
|
|
| Cor
|
|

 

[Cor Ligthert [MVP]]

Jay,

| In my opinion is that a must with a datareader where a connection can
give
| trouble.

Why is the Catch "a must"?

What do you do if you are reading 100000 rows and at 50000 the server goes
down?

I assume that the datareader is returning row by row and not one complete
resultset in a time, if that is the siutation than the catch is not
necessary.

Cor

 

[Bob]

Hi All,
Thanks for your replies.
I went with the inital assignment of both the command object and the
datareader to nothing.
I have also protected the disposals in the finally with a nothing test.
regards
Bob

 

[Jay B. Harlow [MVP - Outlook]]

Cor,
| What do you do if you are reading 100000 rows and at 50000 the server goes
| down?
"Nothing", other then abort the entire transaction.

That's my point, there is nothing you really can do for the read itself,
other then cause what called the read to fail, & possibly cause what called
that & so on...


What do you do?

--
Hope this helps
Jay [MVP - Outlook]
..NET Application Architect, Enthusiast, & Evangelist
T.S. Bradley - http://www.tsbradley.net


| Jay,
|
| > | In my opinion is that a must with a datareader where a connection can
| > give
| > | trouble.
| >
| > Why is the Catch "a must"?
| >
| What do you do if you are reading 100000 rows and at 50000 the server goes
| down?
|
| I assume that the datareader is returning row by row and not one complete
| resultset in a time, if that is the siutation than the catch is not
| necessary.
|
| Cor
|
|

 

[Herfried K. Wagner [MVP]]

Jay,

As the Using statement does your Try/Finally with a check for not Nothing
as
Herfried shows.

I still think that the compiler is stupid.

\\\
Dim f As Form
Try
f = New Form()
Catch ex As Exception

' Warning 1: Variable 'f' is used before it has been assigned a
' value. A null reference exception could result at runtime.
If f IsNot Nothing Then
f.Dispose()
End If
End Try
///

The 'f' in front of 'IsNot' is underlined and the warning included as a
comment in the snippet above is shown. However, it's impossible that a
'NullReferenceException' can be thrown at this line.

I am not specialized in what a compiler is able to detect, but I think that
this warning is counterproductive. It leads people to write explicit
assignments ('Dim f As Form = Nothing'), which is not necessary because the
compiler adds the assignment automatically. If there was no 'Using'
statement in VB 2005, the code above is IMO pretty "good practice" and there
it should not cause any warnings.

Cases where a warning makes sense:

\\\
Dim f As Form
f.Show() ' Warning!
///

\\\
Dim f As Form
If False Then
f = New Form()
End If
f.Show() ' Warning!
///

Cases where a warning does not make sense:

\\\
Dim f As Form
If...Then
f = New Form()
End If
If f IsNot Nothing Then
f.Show() ' No warning!
End If
///

 

[Jay B. Harlow [MVP - Outlook]]

Herfried,
| I am not specialized in what a compiler is able to detect, but I think
that
| this warning is counterproductive.
I agree, the little I've used this warning it seems very counter productive.
I suspect I will be turning it off in most of my projects.

I would think the "null warning" check should only be done when you are
accessing an instance member of the class.

I would not expect the warning when comparing the value with various
operators, including but not limited to: Is, IsNot, TypeOf, CType,
DirectCast, TryCast & other overloaded operators.

--
Hope this helps
Jay [MVP - Outlook]
..NET Application Architect, Enthusiast, & Evangelist
T.S. Bradley - http://www.tsbradley.net


| Jay,
|
| > As the Using statement does your Try/Finally with a check for not
Nothing
| > as
| > Herfried shows.
|
| I still think that the compiler is stupid.
|
| \\\
| Dim f As Form
| Try
| f = New Form()
| Catch ex As Exception
|
| ' Warning 1: Variable 'f' is used before it has been assigned a
| ' value. A null reference exception could result at runtime.
| If f IsNot Nothing Then
| f.Dispose()
| End If
| End Try
| ///
|
| The 'f' in front of 'IsNot' is underlined and the warning included as a
| comment in the snippet above is shown. However, it's impossible that a
| 'NullReferenceException' can be thrown at this line.
|
| I am not specialized in what a compiler is able to detect, but I think
that
| this warning is counterproductive. It leads people to write explicit
| assignments ('Dim f As Form = Nothing'), which is not necessary because
the
| compiler adds the assignment automatically. If there was no 'Using'
| statement in VB 2005, the code above is IMO pretty "good practice" and
there
| it should not cause any warnings.
|
| Cases where a warning makes sense:
|
| \\\
| Dim f As Form
| f.Show() ' Warning!
| ///
|
| \\\
| Dim f As Form
| If False Then
| f = New Form()
| End If
| f.Show() ' Warning!
| ///
|
| Cases where a warning does not make sense:
|
| \\\
| Dim f As Form
| If...Then
| f = New Form()
| End If
| If f IsNot Nothing Then
| f.Show() ' No warning!
| End If
| ///
|
| --
| M S Herfried K. Wagner
| M V P <URL:http://dotnet.mvps.org/>
| V B <URL:http://classicvb.org/petition/>
|

 

[David Browne]

Jay,



I still think that the compiler is stupid.

\\\
Dim f As Form
Try
f = New Form()
Catch ex As Exception

' Warning 1: Variable 'f' is used before it has been assigned a
' value. A null reference exception could result at runtime.
If f IsNot Nothing Then
f.Dispose()
End If
End Try
///

The 'f' in front of 'IsNot' is underlined and the warning included as a
comment in the snippet above is shown. However, it's impossible that a
'NullReferenceException' can be thrown at this line.

I am not specialized in what a compiler is able to detect, but I think
that this warning is counterproductive. It leads people to write explicit
assignments ('Dim f As Form = Nothing'), which is not necessary because
the compiler adds the assignment automatically. If there was no 'Using'
statement in VB 2005, the code above is IMO pretty "good practice" and
there it should not cause any warnings.

No. The above code is and always was bad practice. Declaring local
variables without assigning them immediately is rarely the right thing to
do. Here the code should be:

Dim f As new Form()
Try
'use f
Catch ex As Exception

f.Dispose()
End Try

The Nothing check is not required because f is never Nothing. Only after
sucessfully instantiating a Form does the program need to enter the Try
block. Instantiating the Form inside the Try block only forces the extra
complexity of checking if it is _still_ nothing.

David

 

[Jay B. Harlow [MVP - Outlook]]

David,
Err, Oops...

Try:

| Dim f As new Form()
| Try
| 'use f

Finally

| f.Dispose()
| End Try

As you want to Dispose of the variable even if (*especially if*) an
exception is NOT thrown!

However I will continue to use the code Herfried showed (which is what VB
2005's Using statement creates) as sometimes the constructor itself may
throw the exception in which your code misses, while Herfried & the Using
statement catches.

Of course you could go with 2 Try statements, but that seems like
overkill...

--
Hope this helps
Jay [MVP - Outlook]
..NET Application Architect, Enthusiast, & Evangelist
T.S. Bradley - http://www.tsbradley.net


message |
| | > Jay,
| >
| >> As the Using statement does your Try/Finally with a check for not
Nothing
| >> as
| >> Herfried shows.
| >
| > I still think that the compiler is stupid.
| >
| > \\\
| > Dim f As Form
| > Try
| > f = New Form()
| > Catch ex As Exception
| >
| > ' Warning 1: Variable 'f' is used before it has been assigned a
| > ' value. A null reference exception could result at runtime.
| > If f IsNot Nothing Then
| > f.Dispose()
| > End If
| > End Try
| > ///
| >
| > The 'f' in front of 'IsNot' is underlined and the warning included as a
| > comment in the snippet above is shown. However, it's impossible that a
| > 'NullReferenceException' can be thrown at this line.
| >
| > I am not specialized in what a compiler is able to detect, but I think
| > that this warning is counterproductive. It leads people to write
explicit
| > assignments ('Dim f As Form = Nothing'), which is not necessary because
| > the compiler adds the assignment automatically. If there was no 'Using'
| > statement in VB 2005, the code above is IMO pretty "good practice" and
| > there it should not cause any warnings.
| >
|
| No. The above code is and always was bad practice. Declaring local
| variables without assigning them immediately is rarely the right thing to
| do. Here the code should be:
|
| Dim f As new Form()
| Try
| 'use f
| Catch ex As Exception
|
| f.Dispose()
| End Try
|
| The Nothing check is not required because f is never Nothing. Only after
| sucessfully instantiating a Form does the program need to enter the Try
| block. Instantiating the Form inside the Try block only forces the extra
| complexity of checking if it is _still_ nothing.
|
| David
|
|

 

[David Browne]

David,
Err, Oops...

Try:

| Dim f As new Form()
| Try
| 'use f

Finally

| f.Dispose()
| End Try

As you want to Dispose of the variable even if (*especially if*) an
exception is NOT thrown!

Yes, I didn't want to change the posted code, point out that variables
Disposables should be instantiated before the Try blocks in which they
disposed, not inside them.

David