HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

A more readable InStr: StringContains

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
instrmorestringcontainsreadable

Problem

Consider the following:

If myString = "abc" Or myString = "def" [...] Or myString = "xyz" Then


In C# when myString == "abc" the rest of the conditions aren't evaluated. But because of how VB works, the entire expression needs to be evaluated, even if a match is found with the first comparison.

Even worse:

If InStr(1, myString, "foo") > 0 Or InStr(1, myString, "bar") > 0 [...] Then


I hate to see these things in code I work with. So I came up with these functions a while ago, been using them all over the place, was wondering if anything could be done to make them even better:

StringContains is used like If StringContains("this is a sample string", "string"):

Public Function StringContains(string_source, find_text, Optional ByVal caseSensitive As Boolean = False) As Boolean

    'String-typed local copies of passed parameter values:
    Dim find As String, src As String
    find = CStr(find_text)
    src = CStr(string_source)

    If caseSensitive Then
        StringContains = (InStr(1, src, find, vbBinaryCompare) <> 0)
    Else
        StringContains = (InStr(1, src, find, vbTextCompare) <> 0)
    End If

End Function


StringContainsAny works in a very similar way, but allows specifying any number of parameters so it's used like If StringContainsAny("this is a sample string", false, "foo", "bar", string"):

```
Public Function StringContainsAny(string_source, ByVal caseSensitive As Boolean, ParamArray find_strings()) As Boolean

'String-typed local copies of passed parameter values:
Dim find As String, src As String, i As Integer, found As Boolean
src = CStr(string_source)

For i = LBound(find_strings) To UBound(find_strings)
find = CStr(find_strings(i))
If caseSensitive Then
found = (InStr(1, src, find, vbBinaryCompare) <> 0)
Else
found = (InStr(1, src, find, vbTextCompare) <> 0)
End If
If found Then Exit For
Next

StringContainsAny = found

En

Solution

My 2 cents,

the first function seems fine, you could make it a little DRYer by just setting the compareMethod in your if statement and then have only 1 complicated line of logic. And if you are doing that, you might as well put the Cstr's there.

Public Function StringContains(haystack, needle, Optional ByVal caseSensitive As Boolean = False) As Boolean

   Dim compareMethod As Integer

    If caseSensitive Then
        compareMethod = vbBinaryCompare
    Else
        compareMethod = vbTextCompare
    End If
    'Have you thought about Null?
    StringContains = (InStr(1, CStr(haystack), CStr(needle), compareMethod) <> 0)

End Function


Notice as well that I love the idea of searching for needles in haystacks, I stole that from PHP.

For StringContainsAny, you are not using the code you wrote for StringContains, you repeat it. If you were to re-use the first function, you could do this:

Public Function StringContainsAny(haystack, ByVal caseSensitive As Boolean, ParamArray needles()) As Boolean

    Dim i As Integer

    For i = LBound(needles) To UBound(needles)
        If StringContains(CStr(haystack), CStr(needles(i)), caseSensitive) Then
          StringContainsAny = True
          Exit Function
        End If
    Next

    StringContainsAny = False 'Not really necessary, default is False..

End Function


For the last one I wanted to you consider passing values that you will convert as ByVal, since you are going to make a copy anyway of that variable.

Public Function StringMatchesAny(ByVal string_source, ParamArray potential_matches()) As Boolean

  string_source = CStr(string_source)

  ... 'That code taught me a new trick ;)

End Function

Code Snippets

Public Function StringContains(haystack, needle, Optional ByVal caseSensitive As Boolean = False) As Boolean

   Dim compareMethod As Integer

    If caseSensitive Then
        compareMethod = vbBinaryCompare
    Else
        compareMethod = vbTextCompare
    End If
    'Have you thought about Null?
    StringContains = (InStr(1, CStr(haystack), CStr(needle), compareMethod) <> 0)

End Function
Public Function StringContainsAny(haystack, ByVal caseSensitive As Boolean, ParamArray needles()) As Boolean

    Dim i As Integer

    For i = LBound(needles) To UBound(needles)
        If StringContains(CStr(haystack), CStr(needles(i)), caseSensitive) Then
          StringContainsAny = True
          Exit Function
        End If
    Next

    StringContainsAny = False 'Not really necessary, default is False..

End Function
Public Function StringMatchesAny(ByVal string_source, ParamArray potential_matches()) As Boolean

  string_source = CStr(string_source)

  ... 'That code taught me a new trick ;)

End Function

Context

StackExchange Code Review Q#30816, answer score: 11

Revisions (0)

No revisions yet.