[2005] Problem With If-else Condition

[noam309]

[B]
HI ,I have this code:
the user choose item from the list (ex-"Colors") , and pressed button
the program generate a line from that text file which contains 2 word each like this file:

RED adom
Yellow zahov
white lavan
Green yarok
etc..

(the two words suppose to be with different languages).and displays the first word in textbox1, the user enter a word in textbox2 , and if it's the same as word2 it gives him a point.
THE CODE:

Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        If ListBox1.SelectedIndex() Then
Bk:         If TextBox2.Text = String.Empty Then
                Dim num As New Random
                Dim lines As String() = File.ReadAllLines("D:\Words\" & ListBox1.SelectedItem & ".txt", FileEncoding)
nextrnd:        Dim Line As String = lines(Integer.Parse(num.Next(0, UBound(lines) + 1)))
                Dim Pos As Integer = Line.IndexOf(" ")
                Dim Spted = Split(Was, ",")
                If UBound(Spted) >= UBound(lines) + 1 Then
                    Dim RightA = (Label9.Text / (Int(Label9.Text) + Int(Label10.Text)))
                    Dim Grade = (RightA * 100)
                    MsgBox("You Finished this test!" & vbCrLf & "Your Grade is: " & Int(Grade), MsgBoxStyle.Information, "Done!")
                    Exit Sub
                End If
                If InStr(Was, "," & Line.Substring(0, Pos)) Then GoTo NextRnd
                TextBox1.Text = Line.Substring(0, Pos)
                strAnswer = Line.Substring(Pos + 1, Line.Length - Pos - 1)
            Else
                If TextBox2.Text = strAnswer Then
                    ''    MessageBox.Show("That is Correct!", "Correct", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
                    Label3.Text = "RIGHT!"
                    Label3.ForeColor = Color.Green
                    Label9.Text += 1
                Else
                    ''  MessageBox.Show("I am Sorry, that answer is not correct!", "Incorrect", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
                    Label3.Text = "WRONG!"
                    Label3.ForeColor = Color.Red
                    Label10.Text += 1
                End If
                Label5.Text = TextBox1.Text
                Label4.Text = strAnswer
                Was &= "," & TextBox1.Text
                TextBox2.Text = String.Empty : GoTo Bk
            End If
 
        End If
END sub

there are two problems with this code:
1. if I press on an item (file) in the listbox and it contains 2 words
(ex-"light colors") , it doesn't generate the words into the textbox.
2.only one or two cells in the listbox function OK .. but item1 and 2 and even more aren't function.
PLEASE YOUR HELP ...

[Shaggy Hiker]

A few things.

1) If ListBox1.SelectedIndex() Then should be If ListBox1.SelectedIndex()>-1 Then

2) Dim RightA = (Label9.Text / (Int(Label9.Text) + Int(Label10.Text)))
You really should convert the label9.Text into a numeric type before you do the math. If you had Option Strict on, though it would take more typing, this kind of thing would go away. Also on this line, can you be certain that the two labels in the denominator do not add up to 0? That would be a divide by 0 situation.

3) If InStr(Was, "," & Line.Substring(0, Pos)) Then GoTo NextRnd
Let me be the first to express my horror at the use of GoTo. It still exists in this language, but you won't find anybody in this forum who supports its use. Better by far if it had been left out of .NET. Re-writing as a loop would be better. Search this forum on GoTo to find an explanation of why this is bad. Search the VB6 forum if you want to find a more emphatic discussion.

5) Label9.Text += 1
Again, you are adding an integer to a string. In this case, you could also be concatenating if the compiler interprets this wrong.

6) Those GoTos have bent my mind. I would have to really study that code to figure out how those loops work, and whether they are correct. For me, this was a good lesson in why not to use loops. I understand what you are trying to do, but it is far from obvious to me whether or not it is correct.

7) The major problem appears to be that Pos is giving you trouble. Your code looks for the first blank in the string, which will only work if the string has only one blank. This will not work the same for "light color" as it works for "color". It appears then that you will get different behavior from your first example to your second. I have the feeling that this may be the major source of your error, but those loops are confusing me.

[noam309]

thanks for your reply ,but I still don't know how to fix it, how can I do these loops you were talking about?! and how should I get over the space problem between the 2 words , I need to fix this code a little , every suggest according to the following actions it suppose to do ,will be good.
thanks,
Eyal

[Shaggy Hiker]

The spaces issue is a tough one. Is there ever a case where there might be three words? If there are never more than two, then I could suggest a way around that. Even if there are three, I might be able to suggest a way around it, but it gets increasingly awkward VERY fast.

Could you post an example of what Line might hold. Perhaps the one with the most words?

As for the Goto, if you put in a loop control something like:

<just before the line which you have labeled nextrnd:>
Dim Flag as boolean=True
Do


Then replace these lines:

If InStr(Was, "," & Line.Substring(0, Pos)) Then GoTo NextRnd
                TextBox1.Text = Line.Substring(0, Pos)
                strAnswer = Line.Substring(Pos + 1, Line.Length - Pos - 1)

with:

[Highlight=VB]
'Just added a Not, and killed the GoTo.
If Not InStr(Was, "," & Line.Substring(0, Pos)) Then
'If this If is not entered, Flag will remain True, and execution will return to the Do.
TextBox1.Text = Line.Substring(0, Pos)
strAnswer = Line.Substring(Pos + 1, Line.Length - Pos - 1)
Flag=False 'This will cause the Do...While loop to exit.
End If
Loop While Flag

That would get rid of the first GoTo. The second one is not so simple. While you could replace it with a simple Do Loop and another boolean variable to control it, that is not the solution I would choose. In fact, after thinking about it for a minute, I would choose to take everything that is in that first If statement, and put it in a separate Sub(). Then I would call the Sub in the if statement, and call it in place of the second GoTo. This is easy to say, but would be a monster to show as an example, and I really have to run. Does it help?

The spaces are the bigger issue, however, you can think about the GoTo stuff later.

[<ABX]

That has gotta be one of the most confusing bits of code i've seen.

1) Name your controls (Listbox1 = WordList, Textbox1 = QuestionWord, TextBox2 = AnswerWord)
2) Don't hard code file paths
3) Try and keep your presentation logic separated from your Program logic.

First, as I understand it, what you are attempting to make is a game/test that reads a list of words from files in a directory. Each line in the file has two words the first word is the word in one language and the second is the word in a different language. You want to quiz the user based on this information randomly.

You seem to be awarding 100 points for every correct answer.

(I'm working without Visual Studio here so your results may vary :/)

Option Strict On
Option Explicit On
 
Public Class Form1
 
    Dim rnd As New System.Random()
 
    Private Const WordFilePath As String = "C:\My Documents\Visual Studio 2005\Projects\Quiz\Quiz\Words"
    Private Const WordFileFormat As String = WordFilePath & "\{0}.txt"
 
    'Contains the List of words
    Dim CurrentWordList As ArrayList
    'Contains the current Question
    Dim CurrentQuestion As Question
    Dim Correct As Integer ' A count of how many correct questions
    Dim Incorrect As Integer ' a count of the incorrect questions
 
    'Simple Structure to contain the question
    Private Structure Question
        Dim Question As String
        Dim Answer As String
 
        Public Shared Function ToQuestion(ByVal Question As String, ByVal Answer As String) As Question
            Dim Q As New Question
            Q.Question = Question
            Q.Answer = Answer
            Return Q
        End Function
 
    End Structure
 
    'Loads the Word file into an ArrayList
    
    Private Function ParseWordList(ByVal WordFile As String) As ArrayList
        Dim Words As New ArrayList
        Dim sr As New System.IO.StreamReader(WordFile)
 
        Do While Not sr.EndOfStream
            Dim value() As String = sr.ReadLine().Split(" "c)
            Words.Add(Question.ToQuestion(value(0), value(1)))
        Loop
 
        Return Words
    End Function
 
    Private Function RandomQuestion(ByRef WordList As ArrayList) As Question
        ' this should be moved to global scope
        'This doesn't need any comments *GRIN*
        'Get a random Index
        Dim RandomIndex As Integer = rnd.Next(0, WordList.Count)
        'Get the Random Question
        Dim result As Question = CType(WordList.Item(RandomIndex), Question)
 
        WordList.RemoveAt(RandomIndex)
 
        Return result
    End Function
 
    Private Function ProcessAnswer(ByRef Question As Question, ByVal Answer As String) As Boolean
        If Question.Answer.ToLower = Answer.ToLower Then
            'Correct
            Correct += 1
            Question = Nothing
            Return True
        Else
            Question = Nothing
            Incorrect += 1
            Return False
        End If
    End Function
 
 
    Private Sub Form1_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
        Dim WordLists() As String = System.IO.Directory.GetFiles(WordFilePath, "*.txt")
 
        For Each f As String In WordLists
            Dim fi As New System.IO.FileInfo(f)
            lstWordLists.Items.Add(fi.Name.Substring(0, fi.Name.Length - fi.Extension.Length))
        Next
    End Sub
 
    Private Sub lstWordLists_SelectedIndexChanged(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles lstWordLists.SelectedIndexChanged
        ' This allows the user to change the word list.
        If lstWordLists.SelectedIndex() = -1 Then
            'Nothing Selected, bail
            Exit Sub
        End If
 
        Dim strWordFile As String = String.Format(WordFileFormat, lstWordLists.SelectedItem)
 
        If Not System.IO.File.Exists(strWordFile) Then
            'There is something wrong,
            Exit Sub ' Bail
        End If
 
        'Set the Current Word List
        CurrentWordList = ParseWordList(strWordFile)
    End Sub
 
    'The user clicks this button to start or move to the next question
    Private Sub btnNext_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnNext.Click
 
        ' If there is no current question, load it.
        If IsNothing(CurrentQuestion.Question) Then
            If Not CurrentWordList.Count = 0 Then
                  'New Question...
                   CurrentQuestion = RandomQuestion(CurrentWordList)
                   TextBox1.Text = CurrentQuestion.Question
            Else
                MessageBox.Show("Text Complete!")
            End If
        Else
 
            'I'm assuming there should be no case sensitivity (remove the .ToLower's if not)
            If ProcessAnswer(CurrentQuestion, TextBox2.Text) Then
                'User answered question Correctly
                MessageBox.Show("Correct")
 
                Me.Text = String.Format("Correct: {0}, Incorrect: {1}!", Correct, Incorrect)
                '
                TextBox2.Text = ""
                TextBox1.Text = ""
 
            Else
                Me.Text = String.Format("Correct: {0}, Incorrect: {1}!", Correct, Incorrect)
                MessageBox.Show("Incorrect")
                'User Answered Question Wrong
            End If
 
        End If
 
    End Sub
 
End Class

I'll leave the rest to you but this should give you an idea on how to do it.

Edit: Added code to enumerate the wordlists within a directory.