VS2005: Code "gramatically" correct..

R

Rob Meade

Hi all,

Ok - I've used the word gramatically - which I'm sure is incorect, however I
mean when VS2005 lists "warnings" like this one:

Warning 1 Variable 'Command' is used before it has been assigned a value. A
null reference exception could result at runtime.
Warning 2 Variable 'Connection' is used before it has been assigned a value.
A null reference exception could result at runtime.
Warning 3 Variable 'Command' is used before it has been assigned a value. A
null reference exception could result at runtime.
Warning 4 Variable 'Connection' is used before it has been assigned a value.
A null reference exception could result at runtime.

Now, in my code I often do this within the "catch" part of a try..catch if
I've used a database connection/command in the "Try" section;

' tidy up
If Command Is Nothing <> False Then
Command.Dispose()
Command = Nothing
End If

If Connection Is Nothing <> False Then
Connection.Close()
Connection = Nothing
End If

The reason I do this, I believe is for tidyness, its quite possible that if
an exception was caused I may have already created my connection object,
and/or my Command object, therefore I test to see if they are "something",
and if so deal with them appropriately..

Of course, now with its new gadgets VS reports problems because I've not
"something'd" them in the "Catch" although I have in the "Try"...

Any suggestions - is my coding bad? Is there a better way? Any info
appreciated, I'm trying to go for good coding practices you see :blush:)

Rob
 
M

Marina

One comment, is that when you are testing that the result of a boolean
expression is not equal to false - that is really confusing to read. A more
readable way is:

If Command Is Nothing = True Then

And, better yet:

If Command Is Nothing Then

Which is the most readable out of all of them.

Now, in all of the cases above, your If statement will execute, which is not
what you want, since you will get a nullreferenceexception.

Which proves my point as to just how confusing writing code like that is.

Now, the compiler can only do so much in figuring out your code. It may be,
that your code is written in such a way, that an object is always assigned
before you use it. If you want to get rid of the warning, always assign the
variable, even if it is to nothing.

And lastly, you do not need to set the variables to Nothing. That was
something you had to do in VB6.
 
K

Kevin Spencer

You have a few options:

1. You can ignore warnings which you are certain are not going to cause a
problem. Not a bad idea. After all, a warning is not a show-stopper, but it
does bring your attention to a potential problem.
2. You can turn off certain types of warnings in Tools|Options.
3. You can write code that the IDE prefers you to write. Not a bad idea.
There are good reasons why these warnings are raised, and adapting good
coding practices is always a help.
4. Some combination of the above which suits your specific personality
and/or needs.

--
HTH,

Kevin Spencer
Microsoft MVP
..Net Developer
A watched clock never boils.
 
R

Rob Meade

...
One comment, is that when you are testing that the result of a boolean
expression is not equal to false - that is really confusing to read. A
more readable way is:

If Command Is Nothing = True Then

And, better yet:

If Command Is Nothing Then

Hi Marina,

I understand what you are saying, and thank you for the reply, however, in
my example:

If Command Is Nothing <> False Then
Command.Dispose()
Command = Nothing
End If

I would then have do add more code, ie,

If Command Is Nothing Then
' do nothing
Else
Command.Dispose
Command = Nothing
End If

Easier to read - but more code...
Now, in all of the cases above, your If statement will execute, which is
not what you want, since you will get a nullreferenceexception.

But this is why I'm testing it? Am I not saying in the test, if the object
is "something" then kill it off? ie, if its "nothing" (null?) then dont
worry...skip right past...
Which proves my point as to just how confusing writing code like that is.

I'm obviously missing something, it makes sense to me - is what I've replied
with above here wrong then?
Now, the compiler can only do so much in figuring out your code. It may
be, that your code is written in such a way, that an object is always
assigned before you use it. If you want to get rid of the warning, always
assign the variable, even if it is to nothing.

The full chunk is as follows:

Public Function TruncateDatabase() As Boolean

' declare variables
Dim Connection As SqlConnection
Dim Command As SqlCommand
Dim Result As Boolean

' exception handling
Try

' create and open our database connection
Connection = New
SqlConnection(ConfigurationManager.AppSettings("Webmasters"))
Connection.Open()

' create and define our command object
Command = New SqlCommand
Command.Connection = Connection
Command.CommandType = CommandType.Text
Command.CommandText = "TRUNCATE TABLE Webmasters"

' execute
Command.ExecuteNonQuery()

' tidy up
Command.Dispose()
Command = Nothing
Connection.Close()
Connection = Nothing

' set result
Result = True

Catch ex As Exception

' TODO: Error handling

' tidy up
If Command Is Nothing <> False Then

Command.Dispose()
Command = Nothing

End If

If Connection Is Nothing <> False Then

Connection.Close()
Connection = Nothing

End If

' set flag
Result = False

End Try

' return result
Return Result

End Function

As per my initial post, I only actually set a value to those objects in the
"Try" block, hence the warning...as you suggested, setting them to nothing
at the top outside of Try would obviously clear these up.
And lastly, you do not need to set the variables to Nothing. That was
something you had to do in VB6.

I do that for completeness, I read/heard about .net clearing up after itself
as and when it deems appropriate, I just like to save it the trouble :)

Rob
 
R

Rob Meade

...
1. You can ignore warnings which you are certain are not going to cause a
problem. Not a bad idea. After all, a warning is not a show-stopper, but
it does bring your attention to a potential problem.

I love 'em - I think its one of my favourite things thus far between
2003/2005 - I've always been very task orientated, so for me this is dead
handy, oh look a long list of stuff to sort out :blush:)
2. You can turn off certain types of warnings in Tools|Options.
nah..

3. You can write code that the IDE prefers you to write. Not a bad idea.
There are good reasons why these warnings are raised, and adapting good
coding practices is always a help.

Sounds like a good idea.
4. Some combination of the above which suits your specific personality
and/or needs.

3 for me I think :blush:)

Cheers Kevin,

Rob
 
J

Juan T. Llibre

re:
I understand what you are saying, and thank you for the reply, however, in my example:

If Command Is Nothing <> False Then
Command.Dispose()
Command = Nothing
End If

I would then have do add more code, ie,

Wouldn't :

If Command Then
Command.Dispose()
Command = Nothing
End If

work better ?

In C#, that works, too :

if (Command) {
Command.Dispose();
Command = null;
}



Juan T. Llibre, ASP.NET MVP
ASP.NET FAQ : http://asp.net.do/faq/
Foros de ASP.NET en Español : http://asp.net.do/foros/
======================================
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top