1

I've been writing a code that uses 3 workbooks - but I am having issues with the final output.

  • workbook 1 (wb1 - this workbook - where the macro is run on - and the final code will be displayed)
  • workbook 2 (wb2) which is a customer database for product orders
  • workbook 3 (wb3) which is a reference file for weights (to be manipulated in workbook 2)

wb1 opens up wb2 and wb3, cross-references (using VLOOKUP) the weights in wb3, copies them over to the corresponding customer address in wb2, then multiples the weights by the quantity ordered in wb2's address line.

The entire code works as I planned, except for the final output. wb2 now has the final weights in column Q.

  • All that is left is for the "PO Number" in wb1 (column K) to lookup the multiple "PO Number"s in wb2 (column C as well)
  • Sum wb2's weights (column Q) where there is a match
  • Return that sum back to wb1. I've tried sumif, but to no avail.

Here is the final output code (it returns no values at the moment), with the entire code posted below for reference.

'Enter in the weights data into the final sheet
tempCount = 0
lastCount = lastRow1

For tempCount = 1 To lastCount
    Set lookFor = wb1.Sheets(1).Cells(tempCount + 1, 11) ' value to find
    Set lookForRange = wb2.Sheets(1).Range("$C$2:$C$" & lastRow2)   'Range of values to lookup
    Set srchRange = wb2.Sheets(1).Range("$Q$7:$Q$" & lastRow2) 

wb1.Sheets(1).Activate
    ActiveSheet.Cells(tempCount + 1, 12).Value = Application.WorksheetFuction.SumIf(lookForRange, lookFor, srchRange)
        On Error Resume Next

Next

Below is the entire code for reference.

'Define workbooks
Dim wb2FileName As Variant
Dim wb3FileName As Variant
Dim wb1 As Workbook
Dim wb2 As Workbook
Dim wb3 As Workbook

'Count last rows in columns
Dim lastRow1 As Long
Dim lastRow2 As Long
Dim lastRow3 As Long

'Variables
Dim lookFor As Range
Dim lookForRange As Range
Dim srchRange As Range
Dim tempCount As Integer
Dim lastCount As Integer


'Open up all workbooks to work on
 Set wb1 = ThisWorkbook

wb2FileName = Application.GetOpenFilename(FileFilter:="Microsoft Excel Workbooks (*.csv;*.xls;*.xlsx;*.xlsm),*.csv;*.xls;*.xlsx;*.xlsm", Title:="Customer Order Data Worksheet", MultiSelect:=False)
If wb2FileName <> False Then
    Workbooks.Open Filename:=wb2FileName
End If
    Set wb2 = Workbooks.Open(wb2FileName)

wb3FileName = Application.GetOpenFilename(FileFilter:="Microsoft Excel Workbooks (*.csv;*.xls;*.xlsx;*.xlsm),*.csv;*.xls;*.xlsx;*.xlsm", Title:="Source Reference File (Weights)", MultiSelect:=False)
If wb3FileName <> False Then
    Workbooks.Open Filename:=wb3FileName
End If
    Set wb3 = Workbooks.Open(wb3FileName)

'Find the last row in the customer data workbook and the source weights workbook
wb2.Sheets(1).Activate
    lastRow2 = ActiveSheet.Cells(Rows.Count, 3).End(xlUp).Row

wb3.Sheets(1).Activate
    lastRow3 = ActiveSheet.Cells(Rows.Count, 3).End(xlUp).Row

'Use VLOOKUP to enter in weights from the reference sheet into the customer order data sheet, then multiply by the quantity
tempCount = 0
lastCount = lastRow2

For tempCount = 1 To lastCount
    Set lookFor = wb2.Sheets(1).Cells(tempCount + 6, 10) ' value to find
    Set srchRange = wb3.Sheets(1).Range("$B$2:$C$" & lastRow3)    'source

    wb2.Sheets(1).Activate
        ActiveSheet.Cells(tempCount + 6, 16).Value = Application.WorksheetFunction.VLookup(lookFor, srchRange, 2, False)
        ActiveSheet.Cells(tempCount + 6, 17).Value = ActiveSheet.Cells(tempCount + 6, 11).Value * ActiveSheet.Cells(tempCount + 6, 16).Value
            On Error Resume Next
Next

'Delete top 5 rows from the final sheet and insert new header
wb1.Sheets(1).Activate
    ActiveSheet.Rows("1:5").Delete
    ActiveSheet.Cells(1, 12).Value = "Weights"

'Find the last row on the final sheet
lastRow1 = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row

'Enter in the weights data into the final sheet
tempCount = 0
lastCount = lastRow1

For tempCount = 1 To lastCount
    Set lookFor = wb1.Sheets(1).Cells(tempCount + 1, 11) ' value to find
    Set lookForRange = wb2.Sheets(1).Range("$C$2:$C$" & lastRow2)   'Range of values to lookup
    Set srchRange = wb2.Sheets(1).Range("$Q$7:$Q$" & lastRow2) 


    wb1.Sheets(1).Activate
         ActiveSheet.Cells(tempCount + 1, 12).Value = Application.WorksheetFuction.SumIf(lookForRange, lookFor, srchRange)


Next
  • 1
    Don't ever use `On Error Resume Next` without proper error handing. This line hides all error but they still occur, you just cannot see them. Also a code with that line cannot be debugged because the error are hidden. Remove that line (or implement an error handling instead) and your question might change eventually after removing it. • Also always use `Long` instead of `Integer` [there is no benefit in using integer at all](https://stackoverflow.com/a/26409520/3219613) and Excel has more rows than `Integer` can handle. You cast a `Long` into an `Integer` here: `lastCount = lastRow2` – Pᴇʜ Apr 19 '18 at 15:18
  • Thank you - Noted on the error handling and I have removed it while troubleshooting. Also noted on the `Long` vs. `Integer`. I've made sure that for the output all the variables have the correct information in them (they do - `lookFor`, `lookForRange`,`srchRange`), and when I do the code as an Excel Formula in a cell - it works. I'm still showing blanks though with the output in VBA - despite removing the `On Error Resume Next` – Jason Wingate Apr 19 '18 at 16:04
  • @JasonWingate You have several `On Error Resume Next`s, are you sure you removed them all? Can you update your code block above after you've removed them? – dwirony Apr 19 '18 at 16:11
  • @JasonWingate You also have a lot of unnecessary `Activate`s which make it very hard to debug your code. You can replace all `Activate`s with an explicit sheet reference instead, which will not only improve readability but also your code's performance. – dwirony Apr 19 '18 at 16:15

1 Answers1

1

Okay, I made several additions/changes to your code, so bear with me.

  1. I added Option Explicit to the top of your module (you might already have it but you didn't include your Sub/End Sub so we couldn't tell).

  2. Got rid of Activate & ActiveSheet. This just leads to a plethora of possible errors and a loss in readability. Use explicit references instead.

  3. You need a way to Exit Sub if one of your wb2 or wb3 return False. If they do they'll just throw an error. Now you'll get a MsgBox and the subroutine will exit appropriately.

  4. Got rid of On Error Resume Next. You shouldn't need that here. If you have to use it, at least turn errors back on by using On Error GoTo 0 soon after.

  5. Moved some Sets inside their corrresponding If statements, and moved a couple static Sets outside of a loop (if it's always the same, why put it inside the loop?).

Now, for your issue with the SumIf - I believe you're encountering this issue because your criteria range and your sum range are not the same size. When they aren't, you can get a return of 0 because they don't line up properly. I've changed Range("$Q$7:$Q$" & lastRow2) to Range("$Q$2:$Q$" & lastRow2) in hopes that fixes that (but you might need to change Range("$C$2:$C$" & lastRow2) to Range("$C$7:$C$" & lastRow2) if that's your intended range.

Hope this helps!

Option Explicit
Sub Test()
'Define workbooks
Dim wb2FileName As Variant, wb3FileName As Variant
Dim wb1 As Workbook, wb2 As Workbook, wb3 As Workbook

'Count last rows in columns
Dim lastRow1 As Long, lastRow2 As Long, lastRow3 As Long

'Variables
Dim lookFor As Range, lookForRange As Range, srchRange As Range
Dim tempCount As Integer, lastCount As Integer

'Open up all workbooks to work on
Set wb1 = ThisWorkbook

wb2FileName = Application.GetOpenFilename(FileFilter:="Microsoft Excel Workbooks (*.csv;*.xls;*.xlsx;*.xlsm),*.csv;*.xls;*.xlsx;*.xlsm", Title:="Customer Order Data Worksheet", MultiSelect:=False)

If wb2FileName <> False Then
    Set wb2 = Workbooks.Open(wb2FileName)
Else
    MsgBox "No wb2, exiting"
    Exit Sub
End If

wb3FileName = Application.GetOpenFilename(FileFilter:="Microsoft Excel Workbooks (*.csv;*.xls;*.xlsx;*.xlsm),*.csv;*.xls;*.xlsx;*.xlsm", Title:="Source Reference File (Weights)", MultiSelect:=False)

If wb3FileName <> False Then
    Set wb3 = Workbooks.Open(wb3FileName)
Else
    MsgBox "No wb3, exiting"
    Exit Sub
End If

'Find the last row in the customer data workbook and the source weights workbook
lastRow2 = wb2.Sheets(1).Cells(Rows.Count, 3).End(xlUp).Row
lastRow3 = wb3.Sheets(1).Cells(Rows.Count, 3).End(xlUp).Row

'Use VLOOKUP to enter in weights from the reference sheet into the customer order data sheet, then multiply by the quantity
lastCount = lastRow2

For tempCount = 1 To lastCount
    Set lookFor = wb2.Sheets(1).Cells(tempCount + 6, 10) ' value to find
    Set srchRange = wb3.Sheets(1).Range("$B$2:$C$" & lastRow3)    'source

    wb2.Sheets(1).Cells(tempCount + 6, 16).Value = Application.WorksheetFunction.VLookup(lookFor, srchRange, 2, False)
    wb2.Sheets(1).Cells(tempCount + 6, 17).Value = wb2.Sheets(1).Cells(tempCount + 6, 11).Value * wb2.Sheets(1).Cells(tempCount + 6, 16).Value
Next

'Delete top 5 rows from the final sheet and insert new header
wb1.Sheets(1).Rows("1:5").Delete
wb1.Sheets(1).Cells(1, 12).Value = "Weights"

'Find the last row on the final sheet
lastRow1 = wb1.Sheets(1).Cells(Rows.Count, 1).End(xlUp).Row

'Enter in the weights data into the final sheet
lastCount = lastRow1

Set lookForRange = wb2.Sheets(1).Range("$C$2:$C$" & lastRow2)   'Range of values to lookup
Set srchRange = wb2.Sheets(1).Range("$Q$2:$Q$" & lastRow2)

For tempCount = 1 To lastCount

    Set lookFor = wb1.Sheets(1).Cells(tempCount + 1, 11) ' value to find

    wb1.Sheets(1).Cells(tempCount + 1, 12).Value = Application.WorksheetFuction.SumIf(lookForRange, lookFor, srchRange)

Next

End Sub
dwirony
  • 5,487
  • 3
  • 21
  • 43
  • 1
    Thank you, noted that next time I paste the complete code, I will make sure to include the entire code, including `option explicit`, `sub` and `sub end` Removal of the `Activate` and `ActiveSheet` gave a huge performance boost Regarding the ranges for `SumIf` that was the issue! - After you mentioned that, I checked wb2 again, it was returning too many rows (40), and thus the range ended up being different sizes. The issue was that I had header rows I did not take into consideration (which I changed). – Jason Wingate Apr 19 '18 at 18:38
  • 2
    While you removed the 'On Error Resume Next', I still wanted the code to continue on (and either return a null or ERROR), so I opted to use `Application.Vlookup`, so it would return an ERROR value (N/A) as opposed to going into debug mode, and added an `ifNA` statement to convert any N/A errors into 0. After that - it works great! Thank you! – Jason Wingate Apr 19 '18 at 18:40