If and Else Statements causing multiple messageboxes to show up one after another

Question

I am at a slump here with these If and Else statements, not really my cup of tea as they confuse me. The issue I am experiencing is as follows: I have 5 checkboxes, 1 Button, and a progress bar, and the issue at hand is when checkbox1 is ticked, and you click on button1, you are than infested with Multiple Messageboxes popping up one after another. It doesn't even matter what you tick off in the checkbox, the same sequence of things happens over and over, and I can not for the life of me figure out. How do I correct this? Here is the line of code that I need help resolving.

I want it to check if the file directory exists, and if it doesn't then it downloads the file. If it does exist, then a message comes up once saying "File already exists" and so on and so forth.

Public Sub Server1()
    If (CheckBox1.Checked = True) Then
        If (File.Exists("C:\something1")) Then
            MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S1.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something1"), "C:\something1")
        End If
        If (File.Exists("C:\something2")) Then
            MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
           S2.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something2"), "C:\something2")
        End If
        If (CheckBox2.Checked = True) Then
        ElseIf (File.Exists("C:\something3")) Then
            MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S3.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something3"), "C:\something3")
        End If
        If (File.Exists("C:\something4")) Then
            MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S4.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something4"), "C:\something4")
        End If
        If (CheckBox3.Checked = True) Then
        ElseIf (File.Exists("C:\something5")) Then
            MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S5.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something5"), "C:\something5")
        End If
        If (File.Exists("C:\something6")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S6.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something6"), "C:\something6")
        End If
        If (CheckBox4.Checked = True) Then
        ElseIf (File.Exists("C:\something7")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S7.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something7"), "C:\something7")
        End If
        If (File.Exists("C:\something8")) Then
       MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S8.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something8"), "C:\something8")
        End If
        If (File.Exists("C:\something9")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S9.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something9"), "C:\something9")
        End If
        If (File.Exists("C:\something10")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S10.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something10"), "C:\something10")
        End If
        If (File.Exists("C:\something11")) Then
        MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S11.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something11"), "C:\something11")
        End If
        If (File.Exists("C:\something12")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation) 
        Else
            S12.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something12"), "C:\something12")
        End If
        If (CheckBox5.Checked = True) Then
        ElseIf (File.Exists("C:\something13")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S13.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something13"), "C:\something13")
        End If
        If (File.Exists("C:\something14")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation) 
        Else
            S14.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something14"), "C:\something14")
        End If
        If (File.Exists("C:\something15")) Then
        MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation) 
        Else
            S15.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something15"), "C:\something15")
        End If
        If (File.Exists("C:\something16")) Then
        MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation) 
        Else
            S16.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something16"), "C:\something16")
        End If
        If (File.Exists("C:\something17")) Then
         MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            S17.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something17"), "C:\something17")
        End If
    End If
End Sub

Answer 1

Your problem is that you are checking each file independently of the last, and each If block has it's own Message section.

This is further clouded by the fact that you have the same code repeated over and over. You should move the repeated code into a function and call it. Additionally, you can add your files into an array object of some sort, and use a loop - that way you can add more files in future without having to add blocks - you could even hook it up to some sort of config file, etc, so you can avoid the need for changing the code at all.

Private Sub GetFile(ByVal SourceUrl As String, ByVal TargetFileDirectory As String)
    Dim FileName As String = SourceUrl.Substring(SourceUrl.LastIndexOf("/") + 1)
    Dim TargetFilePath As String = Path.Combine(TargetFileDirectory, FileName)


    If (File.Exists(TargetFilePath)) Then
        MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
    Else
        S1.DownloadFileAsync(New Uri(SourceUrl), TargetFilePath)
    End If
End Function

Public Sub Server1()
    Dim items As New Dictionary(Of CheckBox, String)
    items.Add(CheckBox1, "https://dl.dropboxusercontent.com/something1")
    items.Add(CheckBox2, "https://dl.dropboxusercontent.com/something2")
    items.Add(CheckBox3, "https://dl.dropboxusercontent.com/something3")
    items.Add(CheckBox4, "https://dl.dropboxusercontent.com/something4")
    '..... Add more as required.

    Dim TargetDirectory As String = "C:\"

    For Each chk As CheckBox In items.Keys

        If chk.Checked Then
            Dim url As String = items(chk)

            GetFile(url, TargetDirectory)
        End If

    Next

    If (CheckBox1.Checked = True) Then
        For Each item As String In items
            GetFile(item, TargetDirectory)
        Next
    End If
End Sub

I've taken some liberties with refactoring to make it more generic, but functionally, this is essentially what you are doing at the moment.

It becomes clear now, that each File Get is completely separate from each other, so the messages keep on coming.

Theres a couple of ways to deal with this:

• You could display only the first failure, and stop the download of every other file. But this likely isn't ideal
• You could simply inform the user that some messages already existed
• You could return a list of files that already existed

I'll demonstrate the last:

''' <returns>True if the get was successful</returns>
Private Function GetFile(ByVal SourceUrl As String, ByVal TargetFileDirectory As String) As Boolean
    Dim FileName As String = SourceUrl.Substring(SourceUrl.LastIndexOf("/") + 1)
    Dim TargetFilePath As String = Path.Combine(TargetFileDirectory, FileName)

    Dim retVal As Boolean = False

    If File.Exists(TargetFilePath) = False Then
        S1.DownloadFileAsync(New Uri(SourceUrl), TargetFilePath)
        retVal = True
    End If

    Return retVal
End Function

Public Sub Server1()
    Dim items As New Dictionary(Of CheckBox, String)
    items.Add(CheckBox1, "https://dl.dropboxusercontent.com/something1")
    items.Add(CheckBox2, "https://dl.dropboxusercontent.com/something2")
    items.Add(CheckBox3, "https://dl.dropboxusercontent.com/something3")
    items.Add(CheckBox4, "https://dl.dropboxusercontent.com/something4")
    '..... Add more as required.

    Dim TargetDirectory As String = "C:\"

    Dim ExistingFiles As New List(Of String)

    For Each chk As CheckBox In items.Keys

        If chk.Checked Then
            Dim url As String = items(chk)

            If GetFile(url, TargetDirectory) = False Then
                ExistingFiles.Add(url)
            End If
        End If

    Next

    If ExistingFiles.Count > 0 Then
        Dim msg As String = String.Format("One or more files already exist locally, and have not been downloaded: {0}", String.Join(vbCrLf, ExistingFiles))

        MessageBox.Show(msg, "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
    End If
End Sub

You might alternatively have a textbox (or similar) on your form where you add the Failure messages to, rather than having a messagebox pop up. If you did this, you could add your text to the textbox within the For Loop, rather than waiting until the end.

Update

Based on your requirements from your comment. It appears that you have a checkbox that relates to each individual file. You can handle this by making a tweak to the method to check the checked value, and by changing the List to a Dictionary containing the checkboxes. I have updated the code blocks above to accomodate for this.

Answer 2

From what I can see in your code, your If statements do not look correct for what I think you are wanting to do.

You basically have the first If (CheckBox1.Checked = True) Then wrapping the entire block of code. Your other CheckBox.Checked statements are not actually doing anything.

For your Checkbox2.Checked check, your code looks like this;

If (CheckBox2.Checked = True) Then

    '' do something if true

ElseIf (File.Exists("C:\something3")) Then

    '' regardless of if the CheckBox2 is checked or not it will still go to this check if the file exists

    MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
Else

    '' only if CheckBox2 is not checked then run this code without checking if the file exists

    S3.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something3"), "C:\something3")

End If

'' this next IF block doesnt check for CheckBox2

If (File.Exists("C:\something4")) Then

    MessageBox.Show("some message", "hi", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)

Else

S4.DownloadFileAsync(New Uri("https://dl.dropboxusercontent.com/something4"), "C:\something4")

End If

I think you need to start writing your If statements from scratch, bit by bit, until you are sure it is working correctly, e.g.

If CheckBox1.Checked Then
    '' add your sub If statements within
End If

If CheckBox2.Checked Then
    '' add your sub If statements within
End If

Also you can tidy up your code a bit by using

If CheckBox.Checked Then

Rather than

If (CheckBox.Checked = True) Then

Vb.Net does not require the brackets, also CheckBox.Checked is a boolean value so no need for the = True