VBA: InStr "OR" issue

Question

I am having issue with my InStr "OR" code. This code: If InStr(items, "harmo") Or InStr(items, "pointt") Then count_of_Harmo = count_of_Harmo + 1 is able to execute well (Highlighted in red below) but this one doesn't ElseIf InStr(items, "addi") Or InStr(items, "add-i") Or InStr(items, "plug") Then count_of_Addi = count_of_Addi + 1(Highlighted in blue below). Please don't mind the other results/counts on the image below as i was not able to capture the whole page as it I think it is relevant to the case. Please see the full code below too. Thank you.

Option Compare Text

Public Sub Keywords_Outlook()

Dim row_number As Long
Dim count_of_Harmo As Long
Dim count_of_Room As Long
Dim count_of_Skyp As Long
Dim count_of_Detach As Long
Dim count_of_Wire As Long
Dim count_of_Addi As Long
Dim count_of_crash As Long
Dim count_of_share As Long
Dim count_of_signa As Long
Dim count_of_passw As Long
Dim count_of_follo_and_ops As Long
Dim count_of_follo_and_req As Long
Dim count_of_others As Long
Dim items As Variant
Dim cursht As String 'for the macro to run in any sheet
cursht = ActiveSheet.Name 'for the macro to run in any sheet

row_number = 1
count_of_Harmo = 0
count_of_Room = 0
count_of_Skyp = 0
count_of_Detach = 0
count_of_Wire = 0
count_of_Addi = 0
count_of_crash = 0
count_of_share = 0
count_of_signa = 0
count_of_passw = 0
count_of_follo_and_ops = 0
count_of_follo_and_req = 0
count_of_others = 0
count_of_all = 0

Do

row_number = row_number + 1
items = Sheets(cursht).Range("N" & row_number)

    If InStr(items, "harmo") Or InStr(items, "pointt") Then
        count_of_Harmo = count_of_Harmo + 1
    ElseIf InStr(items, "room") Then
        count_of_Room = count_of_Room + 1
    ElseIf InStr(items, "skyp") Then
        count_of_Skyp = count_of_Skyp + 1
    ElseIf InStr(items, "detach") Then
        count_of_Detach = count_of_Detach + 1
    ElseIf InStr(items, "wire") Then
        count_of_Wire = count_of_Wire + 1
    ElseIf InStr(items, "addi") Or InStr(items, "add-i") Or InStr(items, "plug") Then
        count_of_Addi = count_of_Addi + 1
    ElseIf InStr(items, "crash") Or InStr(items, "car") Then
        count_of_crash = count_of_crash + 1
    ElseIf InStr(items, "share") Then
        count_of_share = count_of_share + 1
    ElseIf InStr(items, "signa") Then
        count_of_signa = count_of_signa + 1
    ElseIf InStr(items, "passw") Then
        count_of_passw = count_of_passw + 1
    ElseIf Trim(items) Like "*followu*" And Trim(items) Like "*ops*" Then
        count_of_follo_and_ops = count_of_follo_and_ops + 1
    ElseIf Trim(items) Like "*followu*" And Trim(items) Like "*req*" Then
        count_of_follo_and_req = count_of_follo_and_req + 1

    ElseIf items <> "" Then
        count_of_others = count_of_others + 1
    End If

Loop Until items = ""

count_of_all = count_of_Harmo + count_of_Room + count_of_Skyp + count_of_Detach + count_of_Wire + count_of_Addi + count_of_crash + count_of_signa + count_of_passw + count_of_follo_and_ops + count_of_follo_and_req + count_of_others

Range("N2").Select


Selection.End(xlDown).Select
lastCell = ActiveCell.Address

ActiveCell.Offset(3, 0).Value = "Count"
ActiveCell.Offset(4, 0).Value = count_of_Harmo
ActiveCell.Offset(5, 0).Value = count_of_Room
ActiveCell.Offset(6, 0).Value = count_of_Skyp
ActiveCell.Offset(7, 0).Value = count_of_Detach
ActiveCell.Offset(8, 0).Value = count_of_Wire
ActiveCell.Offset(9, 0).Value = count_of_Addi
ActiveCell.Offset(10, 0).Value = count_of_crash
ActiveCell.Offset(11, 0).Value = count_of_share
ActiveCell.Offset(12, 0).Value = count_of_signa
ActiveCell.Offset(13, 0).Value = count_of_passw
ActiveCell.Offset(14, 0).Value = count_of_follo_and_ops
ActiveCell.Offset(15, 0).Value = count_of_follo_and_req
ActiveCell.Offset(16, 0).Value = count_of_others
ActiveCell.Offset(18, 0).Value = count_of_all
ActiveCell.Offset(3, 1).Value = "Outlook Keywords"
ActiveCell.Offset(4, 1).Value = "Harmo"
ActiveCell.Offset(5, 1).Value = "Room"
ActiveCell.Offset(6, 1).Value = "Skyp"
ActiveCell.Offset(7, 1).Value = "Detach"
ActiveCell.Offset(8, 1).Value = "Wire"
ActiveCell.Offset(9, 1).Value = "Addi or add-i or plug"
ActiveCell.Offset(10, 1).Value = "Crash"
ActiveCell.Offset(11, 1).Value = "Share"
ActiveCell.Offset(12, 1).Value = "Signa"
ActiveCell.Offset(13, 1).Value = "passw"
ActiveCell.Offset(14, 1).Value = "FollowU and ops"
ActiveCell.Offset(15, 1).Value = "FollowU and req"
ActiveCell.Offset(16, 1).Value = "Others"
ActiveCell.Offset(18, 1).Value = "Total"
ActiveCell.Offset(3, -1).Value = "Percent"
ActiveCell.Offset(4, -1).Value = count_of_Harmo / count_of_all
ActiveCell.Offset(5, -1).Value = count_of_Room / count_of_all
ActiveCell.Offset(6, -1).Value = count_of_Skyp / count_of_all
ActiveCell.Offset(7, -1).Value = count_of_Detach / count_of_all
ActiveCell.Offset(8, -1).Value = count_of_Wire / count_of_all
ActiveCell.Offset(9, -1).Value = count_of_Addi / count_of_all
ActiveCell.Offset(10, -1).Value = count_of_crash / count_of_all
ActiveCell.Offset(11, -1).Value = count_of_share / count_of_all
ActiveCell.Offset(12, -1).Value = count_of_signa / count_of_all
ActiveCell.Offset(13, -1).Value = count_of_passw / count_of_all
ActiveCell.Offset(14, -1).Value = count_of_follo_and_ops / count_of_all
ActiveCell.Offset(15, -1).Value = count_of_follo_and_req / count_of_all
ActiveCell.Offset(16, -1).Value = count_of_others / count_of_all


End Sub

can you try `UCase` to search the items. for eg: `InStr(Ucase(items), "HARMO") Or InStr(Ucase(items), "POINTT")` — nishit dey, Dec 06 '17 at 08:28
@Jonathan read my answer and code below, you should switch to array of counters, and to `Select Case`. — Shai Rado, Dec 06 '17 at 08:58
Incidently, in your original code as structured Excel will look up which cell is the Activecell for every line. If you must use Activecell then using a With/End With structure would reduce the work it's doing considerably — Harassed Dad, Dec 06 '17 at 10:44

GSerg · Accepted Answer · 2017-12-06T08:36:32.280

5

In an entire If/ElseIf statement, each branch can only execute if all preceding branches did not.

The cell circled in blue triggered the Harmo branch and it cannot trigger any other branches.

If you don't want that to happen, make each branch a separate If/End If statement.

On a side note, you should consider explicitly comparing the result of InStr to zero in all cases. It works currently because you are only using Or, but if you decide to make the condition more complex it might stop working.

edited Dec 06 '17 at 08:36

answered Dec 06 '17 at 08:30

GSerg

76,472
17
159
346

Found it. thank you. I missed that part. Need to change my keywords now for searching. Thanks again. – Jonathan Dec 06 '17 at 08:36

score 0 · Answer 2 · answered Dec 06 '17 at 08:52

Looking at your code, there is a much more efficient and shorter (code wise) to go.

Switch your multiple ElseIf with Select Case, and instead of having 14 counters, why not use an array of counters CountofArr ?

See sample code below, you can continue the implementation easily :

Option Compare Text

Public Sub Keywords_Outlook()

Dim row_number As Long, LastRow As Long

' use an array that will have multiple counters
Dim CountofArr() As Long

ReDim CountofArr(0 To 14)

Dim items As Variant
Dim cursht As String 'for the macro to run in any sheet

cursht = ActiveSheet.Name ' for the macro to run in any sheet

With Sheets(cursht)
    LastRow = .Cells(.Rows.Count, "N").End(xlUp).Row ' get last row with data in column "N"

    For row_number = 1 To LastRow
        items = .Range("N" & row_number).Value2

        Select Case True
            Case items Like "*harmo*", items Like "*pointt*"
                CountofArr(0) = CountofArr(0) + 1

            Case items Like "*room*"
                CountofArr(1) = CountofArr(1) + 1

            ' add the rest of your cases below


        End Select
    Next row_number

    ' read the array direclty to the cells
    For row_number = 4 To 18
        .Range("O" & row_number).Value = CountofArr(row_number - 4)
    Next row_number

    ' rest of your code

End With

End Sub

VBA: InStr "OR" issue

2 Answers2