VBA for loop not working

Question

I'm having troubles with my loop. I want to make a worksheet, print it (not build in yet, I know how it works), then delete it. After that proceed to the next j to do the same. But it is relooping the j = 1 to 1, so it's trying to create a second worksheet named "print" and that's not possible.

I have checkboxes with name: CheckBox1, CheckBox2, CheckBox'j'. I want to start with CheckBox1 and end with CheckBox25. If it's true then print the sheet.

I think I need to get rid of the first For:
For Each ctrl In Me.Controls
But I don't know how. Because I need it to specify the variable 'j'.

Private Sub PrintKnop_Click()

    Dim ctrl As MSForms.Control
    For Each ctrl In Me.Controls
        If TypeName(ctrl) = "CheckBox" And Left(ctrl.Name, 8) = "CheckBox" Then
            Dim j As Integer
            j = Mid(ctrl.Name, 9, 2)

            For j = 1 To 1
            'it should be possible to adjust the range.
                If ctrl.Value = True Then
                    Dim ws As Worksheet
                    With ThisWorkbook
                    Worksheets("Veiligheid").Copy _
                    before:=ActiveWorkbook.Sheets("Data")
                    Set ws = ActiveSheet
                    ws.Name = "print"
                    End With

                    'Application.DisplayAlerts = False
                    'Sheets("print").Delete
                    'Application.DisplayAlerts = True
                    'These shouldn't be comments, but if I uncomment it, it won't show the failures.

                End If
            Next

            For j = 2 To 4
                If ctrl.Value = True Then
                    With ThisWorkbook
                    Worksheets("Veiligheid").Copy _
                    before:=ActiveWorkbook.Sheets("Data")
                    Set ws = ActiveSheet
                    ws.Name = "printen"
                    End With

                    'Application.DisplayAlerts = False
                    'Sheets("printen").Delete
                    'Application.DisplayAlerts = True
                End If
            Next
        End If
    Next

End Sub

What is the point of j = Mid(ctrl.Name, 9, 2) if the very next line is using j as the index of a for loop? In any event For j = 1 To 1 will only run once, but it is embedded in a larger loop which can presumably run multiple times. I find the question quite unclear. If you want code to run only once, don't put it in a loop. — John Coleman
– John Coleman, Commented Nov 9, 2017 at 17:19
you want to use a case select instead of for next if you're looking for those values learn.microsoft.com/en-us/dotnet/visual-basic/… — Clyde
– Clyde, Commented Nov 9, 2017 at 18:21

FirmwareRootkits · Accepted Answer · 2017-11-09 17:46:11Z

3

One problem I see here is that you are using the variable j multiple times.

j = Mid(ctrl.Name, 9, 2)
...
For j = 1 to 1
...
For j = 2 to 4
...

The line j = Mid(ctrl.Name, 9, 2) will assign some value to j.

The line For j = 1 to 1 will set j = 1 and loop one time.

The line For j = 2 to 4 will set j = 2 and increment j each loop (runs three times)

Are you sure it is looping on For j = to 1 loop and not just moving on to the second loop?

Sub test()

j = 2 + 3
Debug.Print j

For j = 99 to 99
 Debug.print j
Next

For j = 2 to 4
 Debug.print j
Next

End Sub

This outputs values 5, 99, 2, 3, 4

It might be more obvious when the values are out of numerical order.

answered Nov 9, 2017 at 17:46

FirmwareRootkits

1312 silver badges11 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Steffie Over a year ago

I want to loop for j = 1 to 25 in total. But when j = 1 to 1 I need it to do something. When j = 2 to 4 I want something else to do. Etc.

John Coleman Over a year ago

@Steffie if you want to loop from j = 1 to 25, have a single loop which contains some if statements.

Steffie Over a year ago

But I don't want to make 25 if-statements.

Captain Stone · Accepted Answer · 2017-11-09 17:39:13Z

2

It looks like you have repeating operations in the loop and you are looking for switch-like operation. I guess you mean to parse the number of the CheckBox as the variable j. When you get it, the rest of the loop is something like:

    ... Prepare variables for this loop round ...

    If j = 1 Then
        ... do something ...
    Else
        ... do something else ...
    End If

    ... Put here the part that stays the same regardless the j value ...

And no For-loop is needed in this section.

edited Nov 9, 2017 at 17:39

answered Nov 9, 2017 at 17:28

Captain Stone

4464 silver badges6 bronze badges

1 Comment

Captain Stone Over a year ago

Yes, in case there are many cases, Select Case would be my choice.

Clyde · Accepted Answer · 2017-11-09 19:07:38Z

You may be confusing your For j = loop with if j =

for j = will set your variable equal to the value following it

you'd probably be better off with a select case j statement

Private Sub PrintKnop_Click()
    Dim ctrl As MSForms.Control
    For Each ctrl In Me.Controls
        If TypeName(ctrl) = "CheckBox" And Left(ctrl.Name, 8) = "CheckBox" And ctrl.Value = True Then
            Dim j As Integer
            j = Mid(ctrl.Name, 9, 2)
            Select Case j
                Case 1
                    'it should be possible to adjust the range.
                    Dim ws As Worksheet
                    With ThisWorkbook
                      Worksheets("Veiligheid").Copy _
                        before:=ActiveWorkbook.Sheets("Data")
                      Set ws = ActiveSheet
                      ws.Name = "print"
                    End With
                    'Application.DisplayAlerts = False
                    'Sheets("print").Delete
                    'Application.DisplayAlerts = True
                    'These shouldn't be comments, but if I uncomment it, it won't show the failures.
                Case 2 To 4
                    With ThisWorkbook
                      Worksheets("Veiligheid").Copy _
                        before:=ActiveWorkbook.Sheets("Data")
                      Set ws = ActiveSheet
                      ws.Name = "printen"
                    End With
                    'Application.DisplayAlerts = False
                    'Sheets("printen").Delete
                    'Application.DisplayAlerts = True
            End Select
        End If
    Next
End Sub

Salek · Accepted Answer · 2017-11-09 20:27:05Z

You have 2 loops for same J. If you need your code different things for different J value, I think this solution might help:

ja = ja&"|"&Mid(ctrl.Name, 9, 2)
j = split(ja,"|")
for i = 0 to uBound(j)
if cInt(j(i))=1 then do something
if j(i)>1 AND j(i)<5 then do something 'j=2,3,4
if j(i)>4 AND j(i)<26 then do something 'j=5-25
next

BUT the Mid(ctrl.Name, 9, 2) means you are having TWO symbols, and for CheckBox1 it is "x1", NOT "1". That means, in your code j is x1. You need to rename your checkboxes to two-digit index, like "CheckBox01"

OR, you might add one line more:

j = Mid(ctrl.Name, 9, 2)
IF LEFT(j,1)="x" then j=RIGHT(j,1)
For j = 1 to 25
if j = 1 then....
if j >1 then...
next

This allows you to have only 1 from x1

EDIT Just noticed, that length of "CheckBox1" is 9. You might need to get the checkbox number from right 2 symbols:

j = RIGHT(ctrl.Name,2)

And, get rid of "x":

IF LEFT(j,1)="x" then j=RIGHT(j,1)

Collectives™ on Stack Overflow

VBA for loop not working

4 Answers 4

3 Comments

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related