question about some good practises

  • Thread starter Thread starter Bob
  • Start date Start date
B

Bob

Hi,

I compiled an application for the first time with no errors but following
warnings:
1) variable declaration withoit an 'AS':
Dim def()
2) variable 'myvar' is used before it has been assigned a value:
Dim myvar
......
If wk = 1 Then
myvar = "yes"
ElseIf wk = 2 Then
myvar = "no"
End If
.......
3) unused variable: myvar2


Warning 3) is easy to solve: remove it.
Warning 1): is it important to specify (e.g. dim def() as string)? Can this
cause a run-time error?
Warning 2): is this a problem? Can i leave it like this (i'm sure 'myvar'
will get a value, so no risk of run-time error ...)

Thanks for advice
Bob
 
Bob said:
I compiled an application for the first time with no errors but following
warnings:
1) variable declaration withoit an 'AS':
Dim def()
2) variable 'myvar' is used before it has been assigned a value:
Dim myvar
.....
If wk = 1 Then
myvar = "yes"
ElseIf wk = 2 Then
myvar = "no"
End If
......
3) unused variable: myvar2


Warning 3) is easy to solve: remove it.
Yes.

Warning 1): is it important to specify (e.g. dim def() as string)? Can
this cause a run-time error?
Click to expand...

It's better to specify the type of the array elements because it will enable
the compiler to perform type checking:

\\\
Dim Numbers(...) As Integer
Numbers(0) = Me.TextBox1.Text
///

.... will raise a compile-time error if 'Option Strict' is enabled.
Warning 2): is this a problem? Can i leave it like this (i'm sure 'myvar'
will get a value, so no risk of run-time error ...)
Click to expand...

This warning is IMO rather useless in VB because VB initializes Variables to
specified values implicitly. I typically disable this warning.
 
Bob,

An addition to Herfrieds answer,

It is good investigate these warnings and not to solve them by intializing
with some dumb value as some like to do.

Use values inside the level you need them and don't declare them as done in
the old cobol time in top of a program (this Cobol method is partially
adepted by BASIC people too).

Set option explicit to On and code your program like that.

Cor
 
Thanks both

Cor Ligthert said:
Bob,

An addition to Herfrieds answer,

It is good investigate these warnings and not to solve them by intializing
with some dumb value as some like to do.

Use values inside the level you need them and don't declare them as done
in the old cobol time in top of a program (this Cobol method is partially
adepted by BASIC people too).

Set option explicit to On and code your program like that.

Cor
Click to expand...
 
Herfried K. Wagner said:
It's better to specify the type of the array elements because it will
enable the compiler to perform type checking:

\\\
Dim Numbers(...) As Integer
Numbers(0) = Me.TextBox1.Text
///

... will raise a compile-time error if 'Option Strict' is enabled.


This warning is IMO rather useless in VB because VB initializes Variables
to specified values implicitly. I typically disable this warning.
Click to expand...

I agree that it is mostly useless but in this case it makes sense since the
variable in question is a string and hence gets initalized to Nothing. The
if statement in Bob's post doesn't assign a value to it if wk is anything
else than 1 or 2 so there is a high potentional for a
NullReferenceException. Bob didn't post the code where the compiler
complains though so hard to tell if there might be a problem. Consider the
following:

Dim wk As Integer = 5
Dim myvar As String
If wk = 1 Then
myvar = "yes"
ElseIf wk = 2 Then
myvar = "no"
End If
myvar = myvar.Trim()

The compiler complains on the last row and if you ignore it you will get a
nice null reference exception when you run it.

/claes
 
Claes,

And in your solution probably an unwanted result?

I prefer the error.

Cor
 
Hello Cor Ligthert [MVP],

I disagree with Cor on this point. Initializing a variable at instantiation
is something I really like.

Dim tValue As String
Dim tValue As String = String.Empty

Of the two statements above I prefer the latter. This guarantees that you
will actuially have a string (instead of Nothing).. it avoids the compiler
initialization warning.. and it makes it so you don't have to guess what
the variable holds since you initialize it right off the bat with a value
of your choosing. More than that.. the first statement has an unfinished
feel to it that makes my skin crawl.. like the programmer was lazy and did
a half-ass job.

-Boo
 
My sample code was to illustrate the danger in ignoring the warning. I don't
turn off the warning.

/claes
 
Boo,

Really nice as you want it.

Dim employeecode as integer = 0
dim ammount = 10000
If employeecode = 1 then
salary = ammount * 10
Elseif employeecode = 2 then
salary = ammount * 20
End if
Dim TheSalaryOfBooBoo as integer = salary

It throws no error, but I think it is what you want, I have another opinion

:-)

Cor
 
Hello Cor Ligthert [MVP],

Well, to my eyes there are four things wrong with your example.. Let's not
forget we're in a discussion about best practices and personal preferences.

First, you forgot (or did not) declare salary and initialize it.
Second, when testing for multiple values in a single variable you MUST ALWAYS,
regarless of your preference in instantiation initializers, account for the
unknown, even if your response to unknown is to throw an error.
Third, you forgot to declare the type of ammount.
Fourth, the constants 1 and 2 do not mean anything to me.. an enum is more
readable.
I also prefer Select Case to ElseIf..So the example becomes:

Public Enum EmployeeCode
Grunt = 0
MiddleManager = 1
End Enum

Dim tEmployeeCode As EmployeeCode = EmployeeCode.Grunt
Dim tAmount As Decimal = 10000
Dim tSalary As Decimal = 0
Dim tTheSalaryOfBooBoo As Decimal = tSalary

Select Case tEmployeeCode
Case EmployeeCode.Grunt
tSalary = tAmount * 10

Case EmployeeCode.MiddleManager
tSalary = tAmount * 20

Case Else
Throw New Execption("DANGER WILL ROBINSON! DANGER! Unrecognized
Employee Code.")

End Select

tTheSalaryOfBooBoo = tSalary


-Boo
 
Boo,

I knew that it was wrong, and I assume that you know that I could have done
it right well. It was only meant to show what could happen if you did
initialize and your program was wrong and than nothing was showed. In fact
did you commit that with your corrected code.

Beside that I see not any reason to initialize because it takes only some
instruction extra and that is all, it is nothing more than killing a
warning.

Cor
 
Back
Top