3

This code produces an error when there is no file path in the import tab. Therefore, I included On Error Resume Next in order to run the next loop. However, after On Error Resume Next the code continues to run through the copy operation which messes up the tab I'm copying to.

I identified that the solution is that On Error the code should enter into the next loop instead of continuing the operation. Does anyone have any input on how to change the Error handling to do that?

Sub ImportBS()

Dim filePath As String
Dim SourceWb As Workbook
Dim TargetWb As Workbook
Dim Cell As Range
Dim i As Integer
Dim k As Integer
Dim Lastrow As Long


'SourceWb - Workbook were data is copied from
'TargetWb - Workbook were data is copied to and links are stored

Application.ScreenUpdating = False

Set TargetWb = Application.Workbooks("APC Refi Tracker.xlsb")
Lastrow = TargetWb.Sheets("Import").Range("F100").End(xlUp).Row - 6


    For k = 1 To Lastrow
    

        filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
        Set SourceWb = Workbooks.Open(filePath)
    
    On Error Resume Next
        Range("A1").CurrentRegion.Copy
        TargetWb.Sheets("Balance Sheet Drop").Range("D" & 2 + (k - 1) * 149).PasteSpecial Paste:=xlPasteValues
        Range("A1").Copy
        Application.CutCopyMode = False
        SourceWb.Close

    Next

Application.ScreenUpdating = True

Worksheets("Import").Activate

    MsgBox "All done!"

End Sub
SandPiper
  • 2,816
  • 5
  • 30
  • 52
Juli44
  • 41
  • 7
  • You are assuming that `APC Refi Tracker.xlsb` is already open. Is your code located in it or is it in a third workbook? You are using `F100`. Is there any data below the 100th row? If this `Range("A1").CurrentRegion.Copy` is referring to the `ActiveSheet` in the just opened Source Workbook, what is its name or index? – VBasic2008 Dec 25 '20 at 16:32
  • 1
    In VBA, if you are looking for a structure that allows you to 'continue' from the 'next' statement in a for loop, you are usually looking at moving code into a seperate function and using guard statements in that function that reverse the logic of the tests that are forcing the attempt to continue. – freeflow Dec 25 '20 at 18:37
  • @VBasic2008 APC Refi Tracker.xlsb is the host of the code and is open at the time of operation. The Import Tab is in Refi Tracker and Hosts the links, however often there is a situation where I don't have a link for this particular deal and therefore the purpose would be skipping that field. As this would produce an error in the code I therefore added on Error Resume Next, which is probably not a great way of error handling. – Juli44 Dec 25 '20 at 20:06
  • That leaves the Source worksheet name or index (in my solution referred as `srcID`) as the only unclarified 'variable'. It is important because if you save one or more of the Source workbooks with another than the expected worksheet active, your code may (will) fail. I am referring to the line `Range("A1").CurrentRegion.Copy` whcih should be something like `SourceWb.Worksheets("Sheet1").Range("A1").CurrentRegion.Copy`. – VBasic2008 Dec 25 '20 at 20:21

4 Answers4

5

I would do this differently. I would check for the path using Dir function and then decide what to do. here is an example

For k = 1 To Lastrow
    filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
    
    '~~> Check if the path is valid
    If Not Dir(filePath, vbNormal) = vbNullString Then
        Set SourceWb = Workbooks.Open(filePath)
        
        '
        '~~> Rest of your code
        '
    End If
Next
Siddharth Rout
  • 147,039
  • 17
  • 206
  • 250
  • Makes sense. But do you by any chance know why it was implemented this way? – Super Symmetry Dec 25 '20 at 17:45
  • I guess for scenarios where we do not know what the string is. File or Directory? Ex when we get a string that (say from database) and we need to check if it is valid? – Siddharth Rout Dec 25 '20 at 18:06
  • I don't think you should avoid `On Error` e.g. what would happen if `filePath` is a video file? Off course, you could create an array of allowed extensions to use with `Dir` or some string functions, or use the `FileSystemObject` or what not, but it will complicate matters. freeflow's comment could indicate to create a function like `isExcelFile`. – VBasic2008 Dec 25 '20 at 19:11
  • @VBasic2008 Oh definitely. You should always do a proper handling as I have shown [HERE](https://stackoverflow.com/questions/65284481/optimizing-the-vba-code-and-improve-the-performance) So yes, I would do proper handling and I would still do it the way I have shown above :) But then I am sure each one has his own preference. – Siddharth Rout Dec 25 '20 at 19:36
1

EDIT: corrected code

Try this code (credit for the edit super-symmetry who also linked to this post):

Sub ImportBS()
    
    Dim filePath As String
    Dim SourceWb As Workbook
    Dim TargetWb As Workbook
    Dim Cell As Range
    Dim i As Integer
    Dim k As Integer
    Dim Lastrow As Long
    
    
    'SourceWb - Workbook were data is copied from
    'TargetWb - Workbook were data is copied to and links are stored
    
    Application.ScreenUpdating = False
    
    Set TargetWb = Application.Workbooks("APC Refi Tracker.xlsb")
    Lastrow = TargetWb.Sheets("Import").Range("F100").End(xlUp).Row - 6
    
    On Error Resume Next
    
    For k = 1 To Lastrow
        
        
        filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
        
        Set SourceWb = Workbooks.Open(filePath)
        
        If Err <> 0 Then GoTo Error_Handler
        
        Range("A1").CurrentRegion.Copy
        TargetWb.Sheets("Balance Sheet Drop").Range("D" & 2 + (k - 1) * 149).PasteSpecial Paste:=xlPasteValues
        Range("A1").Copy
        Application.CutCopyMode = False
        SourceWb.Close
Leap:
    Next
    
    On Error GoTo -1
    
    Exit Sub
    
    Error_Handler:
    Err.Clear
    GoTo Leap
    
End Sub    

First (erroneous) answer:

If you want to skip part of your code in case of error, you can use something like this:

    For k = 1 To Lastrow
    

        filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
        Set SourceWb = Workbooks.Open(filePath)
    
    On Error GoTo Leap
        Range("A1").CurrentRegion.Copy
        TargetWb.Sheets("Balance Sheet Drop").Range("D" & 2 + (k - 1) * 149).PasteSpecial Paste:=xlPasteValues
        Range("A1").Copy
        Application.CutCopyMode = False
        SourceWb.Close
Leap:
    Next

I suppose that the error should occur on the line Set SourceWb = Workbooks.Open(filePath). In such case you should probably place the line On Error GoTo Leap before the for's opening; this way it will jump to the next cell in case the first one of the list is empty. I'd also recommend to place a On Error GoTo -1 after the for's closing. Like this:

    On Error GoTo Leap
    
    For k = 1 To Lastrow
    

        filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
        Set SourceWb = Workbooks.Open(filePath)
        
        Range("A1").CurrentRegion.Copy
        TargetWb.Sheets("Balance Sheet Drop").Range("D" & 2 + (k - 1) * 149).PasteSpecial Paste:=xlPasteValues
        Range("A1").Copy
        Application.CutCopyMode = False
        SourceWb.Close
Leap:
    Next
    
    On Error GoTo -1
Evil Blue Monkey
  • 2,276
  • 1
  • 7
  • 11
  • Your code will work for the first error. However, if another error occurs inside the loop your code will fail because you haven't told vba that you had dealt with the first error. VBA needs some sort of `Resume` statement to be satisfied that an error has been dealt with. Have a look at [this](https://stackoverflow.com/questions/65063380/handling-recurring-vba-errors-within-subroutine/) post. – Super Symmetry Dec 25 '20 at 16:45
  • You're right. Thank you. I'll edit the answer. – Evil Blue Monkey Dec 25 '20 at 18:01
  • 1
    I don't think that `On Error Goto -1` is viable in [VBA](https://learn.microsoft.com/en-us/office/vba/language/reference/user-interface-help/on-error-statement). Could you please share a link to the documentation, if I'm wrong, or at least share what it is supposed to do. – VBasic2008 Dec 25 '20 at 18:37
  • I included the code above in the spreadsheet. However it still did not skip the copy and paste operation once there was an error. Therefore I made the modification to move `If Err <> 0 Then GoTo Error_Handler` under `Set SourceWb = Workbooks.Open(filePath)`. It leaps correctly once there is an empty link however once there is a change from empty link to link that opens it still produces an 1004 error and does not perform the copy paste operation. – Juli44 Dec 26 '20 at 02:37
  • @VBasic2008: i've got [this](https://stackoverflow.com/questions/14158901/difference-between-on-error-goto-0-and-on-error-goto-1-vba) and it seems to work on VBA. I hope it helps ^^. – Evil Blue Monkey Dec 26 '20 at 22:16
  • @Juli44: thanks for the feedback. Ye, obviously i had to put the if statement after that line. Clearly a fool mistake by me. I'll take a deeper look to the code and see if i can find why it is not working. – Evil Blue Monkey Dec 26 '20 at 22:16
  • Ok, the error was not cleared so it kept skipping the copy procedure. I'll edit the code now. Not sure why there was that 1004 error though. You may try it. – Evil Blue Monkey Dec 26 '20 at 22:33
  • Confirming (proving) that `On Error Goto -1` is not used with [VBA](https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/on-error-statement). It is [Visual Basic](https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/on-error-statement) syntax. Check out the [second answer](https://stackoverflow.com/questions/14158901/difference-between-on-error-goto-0-and-on-error-goto-1-vba/30994055#30994055) to the question you posted in your comment and Chip Pearson's [Error Handling In VBA](http://www.cpearson.com/excel/errorhandling.htm). – VBasic2008 Dec 27 '20 at 06:23
1

Import Data

A Quick Fix

For k = 1 To Lastrow
    filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
    Set SourceWb = Nothing
    On Error Resume Next
    Set SourceWb = Workbooks.Open(filePath)
    On Error GoTo 0
    If Not SourceWb Is Nothing Then
        Range("A1").CurrentRegion.Copy
        TargetWb.Sheets("Balance Sheet Drop").Range("D" & 2 + (k - 1) * 149).PasteSpecial Paste:=xlPasteValues
        Range("A1").Copy
        Application.CutCopyMode = False
        SourceWb.Close
    'Else ' File not found.
    End If
Next

An Improvement

  • Not tested.
  • Adjust (check) the values in the constants section before using the code.

Option Explicit

Sub ImportBS()

    ' Destination Read
    Const rName As String = "Import" ' where file paths are stored.
    Const rFirstRow As Long = 7
    Const rCol As Variant = "F" ' or 6
    ' Destination Write
    Const wName As String = "Balance Sheet Drop" ' where data is copied to.
    Const wFirstCell As String = "D2"
    Const wRowOffset As Long = 149
    ' Source
    Const srcID As Variant = "Sheet1" ' or e.g. 1 ' where data is copied from.
    Const srcFirstCell As String = "A1"
    
    ' Define Destination Worksheets.
    ' Note that if the workbook "APC Refi Tracker.xlsb" contains this
    ' code, you should use 'Set dstWB = ThisWorkbook' instead which would
    ' make the code more readable, but would also allow you to change
    ' the workbook's name and the code would still work.
    Dim dstWB As Workbook: Set dstWB = Workbooks("APC Refi Tracker.xlsb")
    Dim wsR As Worksheet: Set wsR = dstWB.Worksheets(rName)
    Dim wsW As Worksheet: Set wsW = dstWB.Worksheets(wName)
    
    ' Define Last Row in Destination Read Worksheet.
    Dim rLastRow As Long
    With dstWB.Worksheets(rName)
        rLastRow = .Cells(.Rows.Count, rCol).End(xlUp).Row
    End With
    
    ' Declare additional variables to use in the upcoming loop.
    Dim srcFilePath As String  ' Source File Path
    Dim srcWB As Workbook      ' Source Workbook
    Dim rng As Range           ' Source Range
    Dim i As Long              ' Destination Read Worksheet Rows Counter
    Dim k As Long              ' Destination Write Worksheet Write Counter
    
    Application.ScreenUpdating = False
    
    ' Loop through rows of Destination Read Worksheet
    ' (or loop through Source Workbooks).
    For i = rFirstRow To rLastRow
        ' Read Current Source File Path from Destination Read Worksheet.
        srcFilePath = wsR.Cells(i, rCol).Value
        ' Attempt to open Current Source Workbook.
        Set srcWB = Nothing
        On Error Resume Next
        Set srcWB = Workbooks.Open(srcFilePath)
        On Error GoTo 0
        ' If Current Source Workbook was opened...
        If Not srcWB Is Nothing Then
            ' Define Source Range.
            Set rng = srcWB.Worksheets(srcID).Range(srcFirstCell).CurrentRegion
            ' Define Destination First Cell Range.
            k = k + 1
            ' If a worksheet could not be opened and you want to skip
            ' the 149 lines then replace 'k - 1' with 'i - rFirstRow'
            ' in the following line.
            With wsW.Range(wFirstCell).Offset((k - 1) * wRowOffset)
                ' Write values from Source Range to Destination Range.
                .Resize(rng.Rows.Count, rng.Columns.Count).Value = rng.Value
            End With
            ' Close Source Workbook.
            srcWB.Close SaveChanges:=False
        'Else ' Current Source Workbook was not found.
        End If
    Next
    ' Note that there has been no change of the 'Selection' in any
    ' of the worksheets i.e. what was active at the beginning is still active.
       
    ' Save Destination Workbook.
    'dstWB.Save
    
    Application.ScreenUpdating = True
    
    MsgBox "Data sets copied    : " & k & vbLf _
        & "Data sets not copied: " & i - rFirstRow - k, vbInformation, "Success"

End Sub
VBasic2008
  • 44,888
  • 5
  • 17
  • 28
  • Thank you, I tested this it produces an error of subscript out of range in this line `Set rng = srcWB.Worksheets(srcID).Range(srcFirstCell).CurrentRegion`. Do you know how to solve this @VBasic2008 ?The source workbook will change throughout the operation, as workbooks are opened and closed – Juli44 Dec 26 '20 at 02:45
  • Have you changed `srcID` to the actual `name` (or `index`) of the `worksheet` in `Source Workbook`? You are not reading from the `Source workbook`, but from one of its `worksheets`. – VBasic2008 Dec 26 '20 at 03:08
  • Thank you, @VBasic2008 After changing to the actual worksheet name the code is now not producing an error and runs through the operation. However in my case for testing purposes I have I have links in loop 1 to 3 and then I have no links in loop 4 to 6 and 7 is the last link. The only problem left is that the code is now copying the information from 'srcWB' opened in loop 7 to 'dstWB' where data of loop 4 needs to be saved. So the code is missing the counter of the links not opened until loop 7 to copy it in the right destination. – Juli44 Dec 26 '20 at 13:59
  • 1
    In the code look for this comment: *If a worksheet could not be opened and you want to skip the 149 lines then replace 'k - 1' with 'i - rFirstRow' in the following line.* And don't forget to replace `Set dstWB = Workbooks("APC Refi Tracker.xlsb")` with `Set dstWB = ThisWorkbook` for reasons mentioned in the comment before the relevant line of code. – VBasic2008 Dec 26 '20 at 14:03
0

Try this:

For k = 1 To Lastrow
    filePath = TargetWb.Sheets("Import").Range("F" & 6 + k).Value
    Set SourceWb = Workbooks.Open(filePath)

    On Error Resume Next
    Range("A1").CurrentRegion.Copy
    If Err <> 0 Then GoTo ContinuationPoint
    On Error GoTo 0
    TargetWb.Sheets("Balance Sheet Drop").Range("D" & 2 + (k - 1) * 149).PasteSpecial 
    Paste:=xlPasteValues
    Range("A1").Copy
    Application.CutCopyMode = False
    SourceWb.Close

ContinuationPoint:
    On Error GoTo 0
Next

Note two things. I added On Error GoTo 0 in there twice. When you used On Error Resume Next you have essentially turned off error handling. This now turns it back on. If you HAD an error when you tried to copy, then it will just jump to ContinuationPoint (which you can rename to anything you want). Either way, we turn error handling back on.

SandPiper
  • 2,816
  • 5
  • 30
  • 52
  • Thanks @SandPiper I tested your modification - The code now producess a 1004 error, which is caused when `Set SourceWb = Workbooks.Open(filePath)` is blank.I'm not sure why this is happening because the next line says `On Error Resume Next` so it should not be stopping in my opinion. – Juli44 Dec 26 '20 at 03:09
  • @Juli44 that 1004 error will happen if the `filePath` you specified doesn't actually exist. If you move the `On Error Resume Next` to just prior to your set statement then it will continue. It interrupts with a 1004 right there because we turned the error checking back on with our `On Error GoTo 0` command later on. Does that make sense? – SandPiper Dec 26 '20 at 16:11