VB.net Logic Error

Question

This is supposed to be a program that tells how long it would take you to download a given size of file after the user inputs their download speed. intOne doesn't seem to be affected at all by the function that I have created to determine what intOne's value should be. I'm reasonably sure that the equation I'm using is correct.

Public Class Form1

    Private tSize As System.Drawing.Size
    Private checkUserSpeed As Long = 0
    Private checkBitByte As Integer = 0
    Private setSize As Integer = 0
    Private checkUserSize As String = ""
    Private Answer As Double = 0

    Private ReadOnly Property checkDwnldSize() As String
        Get
            Return ComboBox1.Text
        End Get
    End Property

    Function checkDownloadSize(ByVal checkUserSize As String)
        Dim suffixes As New List(Of String)({"b", "k", "m", "g", "t", "p", "e", "z"})
        Dim index As Long = suffixes.IndexOf(checkUserSize.Substring(0, 1).ToLower) > -1
        If index > -1 Then
            Return checkUserSpeed / 1024 ^ index
        Else
            Return False
        End If
    End Function

    Function checkForByte(ByVal checkUserSize)
        If Microsoft.VisualBasic.Right(checkUserSize.ToLower, 7) = "byte(s)" Then
            checkBitByte = 1
            checkDownloadSize(checkUserSize)
            Return True
            End
        Else
            Return False
        End If
        Return checkBitByte
        Return checkUserSpeed
    End Function

    Function checkForBit(ByVal checkUserSize)
        If Microsoft.VisualBasic.Right(checkUserSize.ToLower, 6) = "bit(s)" Then
            checkBitByte = 8
            checkDownloadSize(checkUserSize)
            Return True
            End
        Else
            checkForByte(checkUserSize)
            Return False
        End If
        Return checkBitByte
        Return checkUserSpeed
    End Function

    Function Calculate(ByVal checkUserSpeed, ByVal checkUserSize)
        checkForBit(checkUserSize)

        Return Answer
    End Function

    Private Sub FitContents()
        tSize = TextRenderer.MeasureText(TextBox3.Text, TextBox3.Font)
        TextBox3.Width = tSize.Width + 10
        TextBox3.Height = tSize.Height
    End Sub

    Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        Calculate(Convert.ToInt32(TextBox3.Text), ComboBox3.Text)
        Answer = checkBitByte * ((1024 ^ setSize) / checkUserSpeed)
        TextBox1.Text = Answer
    End Sub

    Private Sub TextBox3_TextChanged(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles TextBox3.TextChanged
        Call FitContents()
    End Sub

    Private Sub Button2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.Click
        Form2.Show()
    End Sub

    Private Sub Button3_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button3.Click
        Form3.Show()
    End Sub
End Class

Help would be greatly appreciated.

Answer 1

You never assign any value to intOne, so it starts with value 0. So this:

intOne = intOne / 1024 ^ 3

is the same as this:

intOne = 0 / 1024 ^ 3

which is of course this:

intOne = 0

As an aside:

• Don't use Dim at the class/form level, use Private.

• Always use As <ReturnType> when decalring you functions

• Turn on Option Strict in your language settings.

• Dim intTwo As Integer = Nothing doesn't do anything useful. Use Dim intTwo As Integer = 0 instead.

• Always declare your variables as a type as well (Dim tSize is missing its type)

---

Followup to your edits:

This:

Function checkForByte(ByVal checkUserSize)

Should be this:

Function checkForByte(ByVal checkUserSize As Long)

Fix this in all places in your code. Everything should have an As DataType. If you turned on Option Explict and Option Strict like was suggested the compiler will locate these problems for you.

Next, do not name local variables and/or parameters the same as your class/form level private variables. I personally prefix my instance variables like this:

Private _checkUserSpeed As Long = 0

Note the underscore. Some people use a lowercase m instead:

Private mCheckUserSpeed As Long = 0

Pick something and be consistent. This makes it clear the scope of a variable.

Also this code is bad, the second Return will never be hit:

Return checkBitByte
Return checkUserSpeed

Finally, assuming you are trying to populate the variable _checkUserSpeed with a value, you are still not ever doing it. Search your code and look for any place you are doing this:

_checkUserSpeed = something

You won't find it. So that variable will always be 0.

None of these are really VB.NET specific issues. All of these best-practices would apply equally to C# or any other langue.

Answer 2

You should look into cleaning up your code first, to improve readability. Avoid code repetition at all costs. If you think you cannot do otherwise, lean towards data repetition instead, i.e. create an XML file to store your values, but keep the code clean. Any errors you find later will then stand out like white crows.

For example, something like this:

Function checkDownloadSize(ByVal strOne As String)
  Dim suffixes As New List(Of String)({"b", "k", "m", "g", "t", "p", "e", "z"})
  Dim index As Long = suffixes.IndexOf(strOne.Substring(0, 1).ToLower) > -1
  If index > -1 Then
    Return intOne / 1024 ^ index
  Else
    Return False
  End If
End Function

roughly replaces your two pages of checkDownloadSize and checkSize.

Notice how every function here is called only once. If you later decide to swap ToLower with your custom implementation, it needs to be done only once. Not only a performance improvement, it also makes your code more maintainable in the long run.

Another point is that I am not using Microsoft.VisualBasic namespace in the above code, and switched to Substring instead. This will ease your burden with migration to C#, and also make native C# devs understand your code better.