Nested Loops Quit after one successful round

Winoria · Jun 27th 2016

Apologies if the thread name is vague, it's the best description I could come up with.

Here's the issue:

I have several loops of code nested together. Their purpose is to transpose the values of cells that are arranged horizontally in one worksheet (FinanceData) into a vertical format in a second worksheet (UtilData). In transposing the data, every account number (UCAC) has 12 rows in the new workbook (UtilData) to correspond to 12 columns in the original workbook (FinanceData). You may have already guessed that the 12 rows / columns correspond to calendar months and the data captures performance within those months. I use both the 'current' account number (CurrentUCAC) and the calendar month (SrchCell) as variables to identify the correct copy/paste cells between workbooks.

Code

Global UtilData As String
Global FinanceData As String
Global ULastCol As Integer
'________________________________


Sub Months()


Dim FLastCol as Integer
Dim FLastRow as Integer
Dim UCAC as String
Dim ULastRow As Integer
Dim FFindCell As Range
Dim SrchCell As String
Dim CurrentUCAC As String


'**** Some set-up code - I am not having problems with this code, just including it for reference****


'Workbooks(UtilData) is the workbook with the updated format. 
'Workbooks(FinanceData) is the workbook with the original data in its original format. This version works great for our finance folk, but not so much for my analyses. 


UtilData = ActiveWorkbook.Name
'Message Box offers to open and identify the new Finance Data file.
    iReply = MsgBox("Please open the new utility data file. The Macro will copy / paste the new data into the existing table.", vbOKCancel, Title:="Launch Transfer")
        If iReply = vbOK Then
        Application.FindFile
        FinanceData = ActiveWorkbook.Name
        End If
        
Workbooks(UtilData).Sheets("Utility Data").Activate
ULastCol = Cells(1, Columns.Count).End(xlToLeft).Column


Workbooks(FinanceData).Activate
FLastCol = Cells(1, Columns.Count).End(xlToLeft).Column
FLastRow = Cells(Rows.Count, 7).End(xlUp).Row


'Use "City Acct Code" as anchors to copy / paste data. There will be a separate row for each month.
ReDim UCAC(2 To FLastRow)
    For i = 2 To FLastRow
        UCAC(i) = Cells(i, 3)
    Next


'***End Set-up Code*********






Workbooks(UtilData).Sheets("Utility Data").Activate
ULastRow = Cells(Rows.Count, 3).End(xlUp).Row
Workbooks(FinanceData).Activate




SrchCell = "July kwh"
For x = 2 To FLastRow
CurrentUCAC = UCAC(x)  'If this looks funky, that's because I use the String in another bank of code not shown here. However, I'm not convinced this is the problem bc the loop never makes it back to this variable. 
    If Cells(x, 3) = CurrentUCAC Then
        


        For y = 7 To 19    'This is the loop that is failing. 
        Set FFindCell = Cells(1, y).Find(SrchCell, LookIn:=xlValues)
            If Not FFindCell Is Nothing Then
            Cells(x, y).Copy
            Workbooks(UtilData).Sheets("Utility Data").Activate
                
                For w = 3 To ULastRow
                    If Cells(w, 3) = CurrentUCAC And Cells(w, 9) = SrchCell Then
                    Cells(w, 11).PasteSpecial (xlPasteValues)
                    Workbooks(FinanceData).Activate
                    FFindCell.Offset(0, 1).Select
                    SrchCell = ActiveCell
                    End If
                Next    'Everything works great until this loop has one successful completion of code (i.e. it successfully copy/pastes values). Then the Sub just ends, and does not finish 'y = 7 to 19' 
            
            End If
        Next   'This is the loop that is failing. I want 'y' to run through columns 7 to 19, currently it's quitting at 8. Column 8 is the first relevant column to the copy/paste method.  
    End If     'This loop is not completed either. 'x' should loop to row 900-ish. If I create breakpoints and run the program slowly, it runs perfect! But it I let it run on its own, at full speed, it does one successful loop and ends. 
Next
    
End Sub

Display More

skywriter · Jun 27th 2016

Re: Nested Loops Quit after one successful round

I don't know exactly what is wrong with your code, but a few suggestions.
I would suggest using long instead of integer when dealing with rows and columns.
Activating workbooks and sheets is unnecessary and slows your code down.
It may also be what is wrong, maybe not, but I would avoid it.
I would suggest using variables for workbooks and sheets.
For example instead of:

Code

Dim wbName as String
wbName = ActiveWorkbook.Name
Workbook(wbName).Activate

Use

Code

Dim wb as Workbook
Set wb = ActiveWorkbook
With wb
  ' Do something here, no need to activate.
End With

I personally like to use Next x or whatever the next refers to as opposed to just Next.
It makes it easier to see what the Next line is referring to.

Now onto your code, each time you see something like:

Code

Cells(x, y).Copy
Cells(w, 11).PasteSpecial (xlPasteValues)

Cells will always refer to the sheet that is active at the time the line of code is running.

Is the sheet you want to be active when these lines of code run the sheet you want to be the active sheet?

This is a big reason to avoid using activate and refer explicitly to a workbook and a worksheet, I would suggest using variables and with statements.

Max1616 · Jun 27th 2016

Re: Nested Loops Quit after one successful round

I think this might be because you activated a new wb (FinanceData) at the end of your loop, so when it moves to the next item in the loop your code is focusing on (FinanceData) and not (UtilData).

Try this:

Code

Global UtilData As String 
Global FinanceData As String 
Global ULastCol As Integer 
 '________________________________
 
Sub Months() 
     
    Dim FLastCol As Integer 
    Dim FLastRow As Integer 
    Dim UCAC As String 
    Dim ULastRow As Integer 
    Dim FFindCell As Range 
    Dim SrchCell As String 
    Dim CurrentUCAC As String 
     
     '**** Some set-up code - I am not having problems with this code, just including it for reference****
     
     'Workbooks(UtilData) is the workbook with the updated format.
     'Workbooks(FinanceData) is the workbook with the original data in its original format. This version works great for our finance folk, but not so much for my analyses.
     
    UtilData = ActiveWorkbook.Name 
     'Message Box offers to open and identify the new Finance Data file.
    iReply = MsgBox("Please open the new utility data file. The Macro will copy / paste the new data into the existing table.", vbOKCancel, Title:="Launch Transfer") 
    If iReply = vbOK Then 
        Application.FindFile 
        FinanceData = ActiveWorkbook.Name 
    End If 
     
    Workbooks(UtilData).Sheets("Utility Data").Activate 
    ULastCol = Cells(1, Columns.Count).End(xlToLeft).Column 
     
    Workbooks(FinanceData).Activate 
    FLastCol = Cells(1, Columns.Count).End(xlToLeft).Column 
    FLastRow = Cells(Rows.Count, 7).End(xlUp).Row 
     
     'Use "City Acct Code" as anchors to copy / paste data. There will be a separate row for each month.
    Redim UCAC(2 To FLastRow) 
    For i = 2 To FLastRow 
        UCAC(i) = Cells(i, 3) 
    Next 
     
     '***End Set-up Code*********
     
     
     
    Workbooks(UtilData).Sheets("Utility Data").Activate 
    ULastRow = Cells(Rows.Count, 3).End(xlUp).Row 
    Workbooks(FinanceData).Activate 
     
     
    SrchCell = "July kwh" 
    For x = 2 To FLastRow 
        CurrentUCAC = UCAC(x) 'If this looks funky, that's because I use the String in another bank of code not shown here. However, I'm not convinced this is the problem bc the loop never makes it back to this variable.
        If Cells(x, 3) = CurrentUCAC Then 
             
             
            For y = 7 To 19 'This is the loop that is failing.
Workbooks(UtilData).Activate 'This is what I added in
                Set FFindCell = Cells(1, y).Find(SrchCell, LookIn:=xlValues) 
                If Not FFindCell Is Nothing Then 
                    Cells(x, y).Copy 
                    Workbooks(UtilData).Sheets("Utility Data").Activate 
                     
                    For w = 3 To ULastRow 
                        If Cells(w, 3) = CurrentUCAC And Cells(w, 9) = SrchCell Then 
                            Cells(w, 11).PasteSpecial (xlPasteValues) 
                            Workbooks(FinanceData).Activate 
                            FFindCell.Offset(0, 1).Select 
                            SrchCell = ActiveCell 
                        End If 
                    Next 'Everything works great until this loop has one successful completion of code (i.e. it successfully copy/pastes values). Then the Sub just ends, and does not finish 'y = 7 to 19'
                     
                End If 
            Next 'This is the loop that is failing. I want 'y' to run through columns 7 to 19, currently it's quitting at 8. Column 8 is the first relevant column to the copy/paste method.
        End If 'This loop is not completed either. 'x' should loop to row 900-ish. If I create breakpoints and run the program slowly, it runs perfect! But it I let it run on its own, at full speed, it does one successful loop and ends.
    Next 
     
End Sub

Display More

Max1616 · Jun 27th 2016

Re: Nested Loops Quit after one successful round

Also Skywriter is 100% correct, follow his advice!

Winoria · Jun 28th 2016

Re: Nested Loops Quit after one successful round

Thanks Max, but the 'search' code should be taking place in the Finance document (to find the data for transposing) before switching over to the new UtilData document.

Skywriter: thanks for the advice, the 'with' method does look like a better method to use. Unfortunately I've already spent more time on this code than I wanted, this portion is just a small part of a larger macro. If I switched methods, I would have to restructure and rewrite most of the macro because it switches between both documents so much. Does the 'activate' method really cause such a massive memory drain that it can prevent code from completing loops?

I will try switching some of the variables from integers to longs, and I'll clean up some of the 'next' statements to clarify them. Maybe something will click into place. Stay tuned!

W

Winoria · Jun 28th 2016

Re: Nested Loops Quit after one successful round

Quote from Winoria;773159

I will try switching some of the variables from integers to longs, and I'll clean up some of the 'next' statements to clarify them. Maybe something will click into place. Stay tuned!

W

Short answer is 'nope'. No change.

Is switching from the '.activate' method to the 'with' method going to make a huge difference in program resource use? I've written a number of macros that switch between workbooks and worksheets using these methods and I've never had this specific problem before. The copy / paste seems to be working fine if the loops run through to the finish.

So for example, if I insert breakpoints and watch the code step-by-step, it works like a champ. But when I remove those breakpoints, it trips up somewhere and ends the sub. No error messages, just acts like running through one round of 'y' variable is enough and done. Even 'y' shows '9' in the locals, though the code clearly stipulates '7 to 19'. For clarity, '8' is the first relevant column where the code ought to kick in.

skywriter · Jun 28th 2016

Re: Nested Loops Quit after one successful round

Look at the following code:

Code

Sub Example()
Dim c As Long


Worksheets("Sheet1").Activate
For c = 1 To 100
    Cells(c, 1).Value = c * 100
    Worksheets("Sheet2").Activate
Next c


End Sub

Display More

I switch worksheets during the very first loop, therefore Cells the very first time the loop runs refers to sheet1, from the second time the loop runs until it ends cells refers to sheet2.

I therefore end up with 1 number on sheet1 and the rest end up on sheet2.

Now keeping this in mind look at your code and read my notes.

Code

At this line of code we see that after the line is executed "Finanace Data" is the
' active workbook.
Workbooks(FinanceData).Activate
     
     
    SrchCell = "July kwh"
    For x = 2 To FLastRow
        CurrentUCAC = UCAC(x)
        'The first time this loop runs when x = 2 cells refers to whatever sheet of whatever workbook is active.
        'We know the first time through the active workbook is "Finance Data"
        If Cells(x, 3) = CurrentUCAC Then
             
             'The first time the above loop runs and x = 2 we know cells in the FFindCell line refers to
             ' "Finance Data" and whatever sheet is active.
            For y = 7 To 19
                Set FFindCell = Cells(1, y).Find(SrchCell, LookIn:=xlValues)
                If Not FFindCell Is Nothing Then
                    Cells(x, y).Copy
                    Workbooks(UtilData).Sheets("Utility Data").Activate
                     
                    For w = 3 To ULastRow
                    ' The first time this loop runs "UtilData" and sheet "Utility Data" is active
                    ' therefore cells in the line below first refers to that workbook and that sheet.
                    ' Once the if statement runs cells below refers to "Finance Data", see my notes below.
                        If Cells(w, 3) = CurrentUCAC And Cells(w, 9) = SrchCell Then
                            Cells(w, 11).PasteSpecial (xlPasteValues)
                            ' Now in the line below you are activating another workbook so the next time this loop runs cells above
                            ' will refer to that workbook.
                            Workbooks(FinanceData).Activate
                            FFindCell.Offset(0, 1).Select
                            SrchCell = ActiveCell
                        End If
                    Next w
                     
                End If
            Next y
    Next x

Display More

Winoria · Jun 28th 2016

Re: Nested Loops Quit after one successful round

OK I gotcha! In my mind this is a nesting problem, specifically that the code switching between workbooks were nested incorrectly. Next time I'll use the "with" method to designate workbooks and switch between them. Should avoid this problem all together.

I updated the code (see below) and got one successful run! I'll cross my fingers that it wasn't just luck. Thanks for your help!

Code

Sub


Workbooks(UtilData).Sheets("Utility Data").Activate
ULastRow = Cells(Rows.Count, 3).End(xlUp).Row
Workbooks(FinanceData).Activate




For x = 2 To FLastRow
SrchCell = "July kwh"
CurrentUCAC = UCAC(x)
    If Cells(x, 3) = CurrentUCAC Then
        For y = 8 To 19
        Set FFindCell = Cells(1, y).Find(SrchCell, LookIn:=xlValues)
            If Not FFindCell Is Nothing Then
            Cells(x, y).Copy
            Workbooks(UtilData).Sheets("Utility Data").Activate
                For w = 3 To ULastRow
                    If Cells(w, 3) = CurrentUCAC And Cells(w, 9) = SrchCell Then
                    Cells(w, 10).PasteSpecial (xlPasteValues)
                    End If
                Next w
            End If
'*****Updated fix****
        Workbooks(FinanceData).Activate
        FFindCell.Offset(0, 1).Select
        SrchCell = ActiveCell
'*****************
        Next y
    End If
Next x
    
End Sub

Display More

skywriter · Jun 29th 2016

Re: Nested Loops Quit after one successful round

My pleasure, thanks for the feedback. :cool:

**norie** · Jun 29th 2016

Re: Nested Loops Quit after one successful round

The problem with activating workbooks/sheets isn't to do with memory drain, activating just isn't needed and slows things down.

It also makes it hard to debug things as it can get kind of confusing as to what's actually active when.

Anyway, here's your code without any activating.

Code

Option Explicit


Global UtilData As String
Global FinanceData As String
Global ULastCol As Long
'________________________________


Sub Months()
Dim wbUtil As Workbook
Dim wbFinance As Workbook
Dim FLastCol As Long
Dim FLastRow As Long
Dim UCAC() As String
Dim ULastRow As Long
Dim FFindCell As Range
Dim SrchCell As String
Dim CurrentUCAC As String
Dim iReply As VbMsgBoxResult
Dim I As Long
Dim X As Long
Dim Y As Long
Dim W As Long


    '**** Some set-up code - I am not having problems with this code, just including it for reference****


    'Workbooks(UtilData) is the workbook with the updated format.
    'Workbooks(FinanceData) is the workbook with the original data in its original format. This version works great for our finance folk, but not so much for my analyses.


    Set wbUtil = ActiveWorkbook    '    UtilData = ActiveWorkbook.Name


    iReply = MsgBox("Please open the new utility data file. The Macro will copy / paste the new data into the existing table.", vbOKCancel, Title:="Launch Transfer")
    
    If iReply = vbOK Then
        Application.FindFile
        Set wbFinance = ActiveWorkbook   ' Data = ActiveWorkbook.Name
    End If


    'Workbooks(UtilData).Sheets("Utility Data").Activate
    ULastCol = wbUtil.Sheets("Utility Data").Cells(1, Columns.Count).End(xlToLeft).Column


    With wbFinance.ActiveSheet    'Workbooks(FinanceData).Activate
        FLastCol = .Cells(1, Columns.Count).End(xlToLeft).Column
        FLastRow = .Cells(Rows.Count, 7).End(xlUp).Row


    'Use "City Acct Code" as anchors to copy / paste data. There will be a separate row for each month.
        ReDim UCAC(2 To FLastRow)
        For I = 2 To FLastRow
            UCAC(I) = .Cells(I, 3)
        Next I
    End With


    '***End Set-up Code*********


    ULastRow = wbUtil.Sheets("Utility Data").Cells(Rows.Count, 3).End(xlUp).Row


    With wbFinance.ActiveSheet


        SrchCell = "July kwh"
        For X = 2 To FLastRow
            CurrentUCAC = UCAC(X)    'If this looks funky, that's because I use the String in another bank of code not shown here. However, I'm not convinced this is the problem bc the loop never makes it back to this variable.
            If .Cells(X, 3) = CurrentUCAC Then


                For Y = 7 To 19    'This is the loop that is failing.
                    Set FFindCell = .Cells(1, Y).Find(SrchCell, LookIn:=xlValues)
                    If Not FFindCell Is Nothing Then
                        .Cells(X, Y).Copy


                        For W = 3 To ULastRow
                            If wbUtil.Sheets("Utility Data").Cells(W, 3) = CurrentUCAC And wbUtil.Sheets("Utility Data").Cells(W, 9) = SrchCell Then
                                wbUtil.Sheets("Utility Data").Cells(W, 11).PasteSpecial (xlPasteValues)
                                SrchCell = FFindCell.Offset(0, 1)
                            End If
                        Next W   'Everything works great until this loop has one successful completion of code (i.e. it successfully copy/pastes values). Then the Sub just ends, and does not finish 'y = 7 to 19'


                    End If
                Next Y    'This is the loop that is failing. I want 'y' to run through columns 7 to 19, currently it's quitting at 8. Column 8 is the first relevant column to the copy/paste method.
            End If    'This loop is not completed either. 'x' should loop to row 900-ish. If I create breakpoints and run the program slowly, it runs perfect! But it I let it run on its own, at full speed, it does one successful loop and ends.
        Next X
        
    End With


End Sub

Display More

Nested Loops Quit after one successful round

Participate now!

Share