How to minimize reusing the same code

B&W

B&W

Soldato
Joined
3 Oct 2003
Posts
7,668
Location
Birmingham
Hi I'm pretty hopeless at VB, basically I have lots of similar code in my program.

I'm pretty sure there must be a way to minimize the repeating of the same code.

The main culprits are two subs with If statements.

Code:
Private Sub ValidateData()

        Dim data As Boolean

        data = IsNumeric(txttemp.Text)

        If data = False Then
            MessageBox.Show("Temp Data is invalid")
            txttemp.Text = ""

        End If

        data = IsNumeric(txtheartrate.Text)

        If data = False Then
            MessageBox.Show("Heart Rate Data is invalid")
            txtheartrate.Text = ""

        End If


Code:
Private Sub paddy()

        If txttemp.TextLength < 4 Then
            txttemp.Text = txttemp.Text.PadLeft(4, "0")

        End If

        If txtheartrate.TextLength < 3 Then
            txtheartrate.Text = txtheartrate.Text.PadLeft(3, "0")

        End If
 
I've never used VB and have no idea of VB's constructs/data types so can't offer advice per se, but you might want to simply make the function return boolean and reset the field, then you can reuse it as a flexible piece of code; pseudocode:

Code:
// No idea as to VB's syntax; this comment is probably even wrong

Private Sub ValidateData(field)

        Dim data As Boolean

        data = IsNumeric(field.Text)

        If data = False Then
            return False
            field.Text = ""
        Else
            return True

        End If

End Sub

If ValidateData(txttemp) = False
	MessageBox.Show("Temp Data is invalid")
End If

If ValidateData(txtheartrate) = False
	MessageBox.Show("Heart Rate Data is invalid")
End If
 
I was always taught never to do:
Code:
If data = False Then
But instead
Code:
If !data Then

The difference is probably lost in compilation anyway and I am only assuming this can be done in vb.

You could also lose data,
Code:
If !IsNumeric(txttemp.Text) Then

But on the basis you need to check this on for each field, it is different for each field and a different error is generated each time I can't see any useful way unless it is an array of components. But I think this is better for readability.
 
Maybe due to the fact in c++ writing someVariable = false will just assign false to someVariable rather than checking for a condition. To do that you need ==. Dunno if its the same in VB though I only program in c++. However the problem comes as = is valid syntax but wont do what you want it to so the debugger wont think anything of it. If you use !condition then its less ambiguous :)
 
Code:
Private Sub ValidateData(data, message)
    If !IsNumeric(data) Then
        MessageBox.show(message)
        Return False
    EndIf

    Return True
End Sub

ValidateData(txttemp.Text, "Temp data is not numeric")
ValidateData(txtheartrate.Text, "Heartrate data is not numeric")
Syntax may be off.. last touched vb6 6 years ago.
 
Dj_Jestar said:
Code:
Private Sub ValidateData(data, message)
    If !IsNumeric(data) Then
        MessageBox.show(message)
        Return False
    EndIf

    Return True
End Sub

ValidateData(txttemp.Text, "Temp data is not numeric")
ValidateData(txtheartrate.Text, "Heartrate data is not numeric")
Syntax may be off.. last touched vb6 6 years ago.
other than returning a boolean from a sub, looks ok here jenk. Then again I'm a vbs monkey..

akakjs
 
VB cannot do If (!Something)... for VB you will need to do If (Not Something)...

TrUz
 
|Ric| said:
I was always taught never to do:
Code:
If data = False Then
But instead
Code:
If !data Then

The difference is probably lost in compilation anyway and I am only assuming this can be done in vb.

You could also lose data,
Code:
If !IsNumeric(txttemp.Text) Then

But on the basis you need to check this on for each field, it is different for each field and a different error is generated each time I can't see any useful way unless it is an array of components. But I think this is better for readability.

If expression = False in VB is fine, in C,C++,C# etc... this will assign False to expression. Instead you would need to use If (expression == False) or If (expression != Flase)... etc....

TrUz
 
Yeh I'm aware it is valid syntax, just saying that I was taught not to write it like that.
And I can see why when you start using other languages, but like I also said any difference is undoubtedly lost at compilation any way so if he finds it easier to read this way then keep doing it.
 
This was back in the days when I was learning pascal. My teachers pascal interpreter we used deliberately didn't accept "if something = true" because he didn't want us using it.
 
Been ages since i did VB but if you wanted something slightly more fancy pants

Code:
For Each objTextBox in Form1.Controls
    If TypeOf objTextBox Is TextBox Then
        If not isNumeric(objTextBox.text) Then
              objTextBox.SelStart = 0
              objTextBox.SelLen = Len(objTextBox.Text)
              MessageBox.Show("Erroneous data")
              break
        End If
    End If
Next

This does of course assume all fields should be numeric, if not you could simply have an array of all the text boxes that require this type of validation and use that instead of form1.controls.
 
|Ric| said:
This was back in the days when I was learning pascal. My teachers pascal interpreter we used deliberately didn't accept "if something = true" because he didn't want us using it.
What a silly teacher imo.

If he/she is trying to enforce good coding practice teach a decent language to start with, rather than disable a perfectly legitamate functionality of the language.
 
I've never seen what the fascination is with Pascal either. Horrible syntax IMO.

But he was right to disable that. "=" on a conditional is like something that "Brain****" language would do :p
 
TrUz said:
If expression = False in VB is fine, in C,C++,C# etc... this will assign False to expression. Instead you would need to use If (expression == False) or If (expression != Flase)... etc....

TrUz

Would it be a better practice to do:

If (False == expression) ?

This would give you a compiler error if you accidentally left out the second equals sign, so you dont accidentally compile the program and let expression get re-assigned :)
 
sirlemonhead said:
Would it be a better practice to do:

If (False == expression) ?

This would give you a compiler error if you accidentally left out the second equals sign, so you dont accidentally compile the program and let expression get re-assigned :)

Alternatively:
Code:
if (!expression)
;)
 
Back
Top Bottom