# Find and display the position of a word in a sentence

How can I simplify this code:

Public Class Form1
Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click
' 'txtSentence.Text = "The cat sat on the mat" ' Temp for testing

MsgBox("The text you are searching is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")
End If

Dim WordArray() = {""}
Dim strSearchWord As String

FindPositionOfWords(strSearchWord, WordArray)

MsgBox("The sentence box is empty, so there are no matches.")
End If

End Sub
Private Sub BuildWordArray(ByVal strText, ByRef strArray)
Dim strTextArray1() = Split(strText, " ")
strArray = strTextArray1
End Sub

Private Sub FindPositionOfWords(ByVal strSearchWord, ByVal strWordList)

Dim intIndex As Integer = 1
Dim strPosn As String = ""
For Each strWord As String In strWordList
If UCase(strWord) = UCase(strSearchWord) Then
If strPosn = "" Then
strPosn = strPosn & Str(intIndex)
Else
strPosn = strPosn & "," & Str(intIndex)
End If
End If
intIndex = intIndex + 1
Next
If strPosn = "" Then
MsgBox("The Search Word Here Box Is Empty", MsgBoxStyle.Exclamation)
Else
MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)
End If
End Sub
End Class


please note I am not that great at vb.net

Variables - Define them! You haven't defined

   TxtAddWord
TxtAddSentence (though I see txtSentence commented out)


are they defined on the form design?

Testing for something to be "" usually has a better way. I think something like this -

If String.IsNullOrEmpty(txtaddsentence) Then


Speaking of that if - you have it twice - why?

If TxtAddSentence.Text = "" Then
MsgBox("The sentence box is empty, so there are no matches.")
End If


I can't see TxtAddSentence changing after the first check as there aren't any functions to return a string.

Speaking of strings, why are you always using

TxtAddSentence.Text


When you can just

    Dim TxtAddSentence = "The cat sat on the mat" ' Temp for testing


Which are string type.

Naming - use variable names to describe what is happening, and avoid that hungarian notation.

Variable -> can be ->
strSearchWord       searchWord
strWordList         what is this? a List Type? An Array Type? A String?
intIndex            index
strPosn             searchPosition
strWord             listedWord


ByVal strText, ByRef strArray)
ByVal strSearchWord, ByVal strWordList


Can be, as far as I can tell

Private Sub FindPositionOfWords(ByVal searchWord As String, ByVal wordArray() As String)
Private Sub BuildWordArray(ByVal sentenceAddition, ByRef wordArray)


It would be best practice to give a type to the variables in BuildWordArray - but I'm not entirely sure what type they should be.

Here -

Dim strPosn As String = ""


If this is position, why isn't it an integer? I would expect an integer, not a string.

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
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 Strcould 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
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
MsgBox("The sentence box is empty, so there are no matches.")
Exit Sub
MsgBox("The search box is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")
Exit Sub
End If
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