There are a few things I would change. Some are performance related(though in your case they wont make any real difference, its good practice though) and others simply reduce the amount of code. Another change should make your code less likely to not find words. OK Here it is.
This line enforces proper programming practices in the case of type conversions and sever other things.
Option Strict On
This line enforces declaration of type rather than having the compiler infer them from other code. You'll get less problems in future programs if you always have this off.
Option Infer Off
This line imports code that allows you to use powerful(though not very readable) text handling tools. See later on.
Imports System.Text.RegularExpressions
Public Class Form1
Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click
the next two lines have been moved to the top of the Class - it's often good practice to have your variables at the top of the definition so they are easier to find. Also, this is the bit that uses Regex as mentioned above in the imports at the top of the code. This line removes everything that is not a space or an alphabetic character. For example, if your sentence is "The cat is big, fat and hairy", if you try searching for "big", using your original code, you won't get a result because the word stored in strWordList
would be "big," not "big"
Dim WordArray() As String = {""}
Dim sentenceToSearch As String = Regex.Replace(TxtAddSentence.Text, "[^a-zA-Z ]", "").ToLower
Check if both of your text boxes have text in. If they don't then there is no need to execute the rest of the code, hence the use of Exit Sub
.
If TxtAddSentence.Text = "" Then
MsgBox("The sentence box is empty, so there are no matches.")
Exit Sub
ElseIf TxtAddWord.Text = "" Then
MsgBox("The search box is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")
Exit Sub
End If
`BuildWordArray is better as a Function than as a Sub for readability and later on in larger programs, maintainability.
WordArray = BuildWordArray(TxtAddSentence.Text)
In this situation however, the sub doesn't change any variables/objects outside of itself, so it is fine to leave it as it is.
FindPositionOfWords(TxtAddWord.Text, WordArray)
End Sub
This function only needs the one line as strTextArray1
only a local variable and you don't really need it in this case as you can directly return the results of the Split
function. Also, this is the point where the string can safely be converted to lower case. Rather than have the program doing several conversions in the search loop, it is only done once here.
Private Function BuildWordArray(ByVal strText As String) As String()
Return Split(strText.ToLower, " ")
End Function
Private Sub FindPositionOfWords(ByVal strSearchWord As String, ByVal strWordList() As String)
See above also for my explanation of Regex
strSearchWord = Regex.Replace(strSearchWord, "[^a-zA-Z ]", "").ToLower
Dim strPosn As String = ""
Rather than use a For Each..Next
loop and still use a variable as an index, just just a For..Next
loop and use it's index. So here there is no definition of `intIndex1 until the loop here.
For intIndex As Integer = 1 To strWordList.Count
See my explanation above about converting strings to lower case earlier on
In the If
statement you now only have the one instead of two If
statements. This will again, in larger programs improve performance. In this code it's simpler just to keep adding the intIndex
variable. The method Str
could be confusre with an array or some other collection, so it is better to use intIndex.ToString
.There is no need in this case to have the extra check with regard to the adding of commas, Just trim off the last comma after the loop has completed.
If strWordList(intIndex) = strSearchWord Then
strPosn = strPosn & intIndex.ToString & ","
End If
Next
strPosn.TrimEnd(","c)
The message box in you original version of the code showed an incorrect message. Earlier on in the new code, the search textbox has already been checked to make sure that it wasn't empty, so the Message should simply be that no results have been found. Telling the user that the box is empty would confuse them.
If strPosn = "" Then
MsgBox("No results found", MsgBoxStyle.Exclamation)
Else
MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)
End If
End Sub
End Class
That's it. To make it easier for you to use the updated code .. here it is in one block
Option Strict On
Option Infer Off
Imports System.Text.RegularExpressions
Public Class Form1
Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click
Dim WordArray() As String = {""}
Dim sentenceToSearch As String = Regex.Replace(TxtAddSentence.Text, "[^a-zA-Z ]", "").ToLower
If TxtAddSentence.Text = "" Then
MsgBox("The sentence box is empty, so there are no matches.")
Exit Sub
ElseIf TxtAddWord.Text = "" Then
MsgBox("The search box is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")
Exit Sub
End If
WordArray = BuildWordArray(TxtAddSentence.Text)
FindPositionOfWords(TxtAddWord.Text, WordArray)
End Sub
Private Function BuildWordArray(ByVal strText As String) As String()
Return Split(strText.ToLower, " ")
End Function
Private Sub FindPositionOfWords(ByVal strSearchWord As String, ByVal strWordList() As String)
strSearchWord = Regex.Replace(strSearchWord, "[^a-zA-Z ]", "").ToLower
Dim strPosn As String = ""
For intIndex As Integer = 1 To strWordList.Count
If strWordList(intIndex) = strSearchWord Then
strPosn = strPosn & intIndex.ToString & ","
End If
Next
strPosn.TrimEnd(","c)
If strPosn = "" Then
MsgBox("No results found", MsgBoxStyle.Exclamation)
Else
MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)
End If
End Sub
End Class