Loop without a Do error

[omnisautnihil]

Hello, I am just now beginning to learn how to write Macros and I have been working on one that is being used to search through one document for a value in another and labeling an adjacent cell with the counter. All of my research points to people who do not have EndIf used appropriately but I do not see how mine is similar. Can anyone see why my code won't compile with "Loop with no Do" error?

*****************************
Sub Sorting()

Windows("File1.xls").Activate
Dim Count As Integer
Count = -1
Dim gCount As Integer
gCount = 0
Dim ICount As Integer
ICount = Count - gCount

Range("D5").Select
Dim rFound As Range

Do Until Selection.Value = ""

Count = Count + 1
Range("D5").Offset(Count, 0).Select
If Selection.Value = "General" Then

gCount = gCount + 1
Loop
​

ElseIf Selection.Value = Selection.Value.Offset(-1, 0) Then

Loop
​

Else

Selection.Value.Copy
Windows("ReviewAide.xls").Activate
Set rFound = Columns(2).Find(What:=Selection.Value, After:=Range ("B832"), LookIn:=xlValues, LookAt:=xlPart, SearchOrder:=xlByRows,_ SearchDirection:=xlNext, MatchCase:=False, SearchFormat:=False)
​

If Not rFound Is Nothing Then

Selection.Offest(0, -1).Select = "Issue " & ICount
ActiveCell.FontStyle = "Bold"
Windows("File1.xls").Activate
​

Else

MsgBox (Selection.Value & "is not a valid ID.")
Exit Do
​

End If
​

End If
​

Loop
MsgBox ("DONE!")
End Sub

**************************************************

If something needs explanation just let me know, but I'm only looking for why it does not understand my Loop statements where I want to break inside the if statements.

 

[MrKowz]

Welcome to the forums!

Just glancing at this code, it appears that there may be no need to use a do-loop at all if we change a few things.

What is this code meant to do?

 

[omnisautnihil]

It's meant to do as stated above. I'm not sure what you mean by change some things. Unfortunately I can't give much more information about the function of the two documents. I guess my main question is how do I go about re-entering a loop before the end of the loop. I assume its just "Loop" is that wrong?

 

[Jonmo1]

Agree with MrKowz about possiblity of doing it without a loop at all....

But I think the problem is that you can only have one Loop for Each Do.

the ENTIRE loop must be contained within the IF
OR
The Entire IF must be contained with the Do Loop.

You can't have an extra Loop (based on results of an if) for the same Do.

 

[omnisautnihil]

So how would i jump back to the beginning of the loop?

 

[MrKowz]

Your code is flawed in several areas, and the logic of it is incredibly difficult to follow without having an idea of what it is meant to do.

For example:

• gCount does not appear to have a purpose, as it is not used in any fashion
• iCount is not being incremented in any way, yet it appears to be used to label an Issue
• Using a Do-Loop in the fashion you have set is incredibly ineffecient. I would recommend using a For loop, that way you don't have to use a counter and an offset to determine what cell to look at.
• It is not good to use Select/Selection statements, as most all code can be used directly on objects.

If you can explain, in detail, the process this is supposed to follow, I am sure I can provide better assistance.

 

[Jonmo1]

So how would i jump back to the beginning of the loop?

You don't have to.
That's the purpose of your IF.

Nothing else is done outside the if..
So the code in the last ELSE is only executed if the criteria of the first 2 if's are not met.

Just remove the 2 extra Loops.
And step through the code using F8
You'll see what happens.

 

[MrKowz]

Based on what I could gather from the code provided, this may work:

Public Sub Sorting()
Dim wb1     As Workbook, _
    ws1     As Worksheet, _
    wb2     As Workbook, _
    ws2     As Worksheet, _
    cCount  As Long, _
    iCount  As Long, _
    gCount  As Long, _
    LR      As Long, _
    i       As Long, _
    rFound  As Range
    
Set wb1 = Workbooks("File1.xls")
Set ws1 = wb1.activeworksheet
Set wb2 = Workbooks("ReviewAide.xls")
wb2.Activate
Set ws2 = wb2.activeworksheet
wb1.Activate
LR = ws1.Range("D" & Rows.Count).End(xlUp).Row
For i = 5 To LR
    If ws1.Range("D" & i).Value = "General" Then
        gCount = gCount + 1
    ElseIf ws1.Range("D" & i).Value = ws1.Range("D" & i - 1).Value Then
    Else
        Set rFound = ws2.Columns(2).Find(What:=ws1.Range("D" & i).Value, After:=Range("B832"), LookIn:=xlValues)
        If Not rFound Is Nothing Then
            With rFound.Offset(0, -1)
                .Value = "Issue " & iCount
                .Font.Bold = True
            End With
        Else
            MsgBox ws1.Range("D" & i).Value & " is not a valid ID."
            Exit For
        End If
    End If
Next i
End Sub

 

[omnisautnihil]

I realize that it is very convoluted and probably the most inefficient way to accomplish my goal. I am not a very good programmer at all. But jonmo1 is correct I believe. I don't know why I thought those If statements were not exclusive.

Now I get a "object Required" error on the line

ElseIf Selection.Value = Selection.Value.Offset(-1, 0) Then

*******************************************************

Here is a breakdown of what the code is supposed to do. I am looking at one data sheet for a product and one spreadsheet of comments from different peer reviewers.

The review sheet is sorted by item number. I begin by evaluating if the item number is just a general comment and skip over it. If it a real number then, I go back to the data sheet and place a "Issue#" in the column next to the item. This number is the counter used.

The line currently giving me an error is to used to evaluate if the previous comment was on the same item. Since this is a compilation of reviewers I don't want to relabel the issue and increment the counter.

Also thank you for your incredibly speedy re-do of my code haha but I'd like to understand why mine doesn't work rather than just observe a whole new approach.

 

[Jonmo1]

Also thank you for your incredibly speedy re-do of my code haha but I'd like to understand why mine doesn't work rather than just observe a whole new approach.

That's the best reason I've ever seen anyone give for sticking with inefficient code. Kudo's.
Usually it's just stubbornness.


ElseIf Selection.Value = Selection.Value.Offset(-1, 0) Then

Should be

ElseIf Selection.Value = Selection.Offset(-1, 0) Then