Learning worksheet functions

[Mark_Annonyous]

I am going through a journey of better understanding VBA, and like all things in life, you feel like your shoveling the proverbial up a hill until you start jumping hurdles.

Well, this is my first worksheet funtion, as the oriignal i wrote, which worked, took to long to run (30 mins)

The error i am getting is method range object: Global fail.

Below is my code, please be nice





Sub Countavol()


Dim DataImportCounter, RowCounter, ColumnCounter, ExShipCount, ImShipCount, LastRowDataImport, LastRow As Integer
Dim ExportCountry, ImportCountry As String
Dim Exportdate As Date

LastRowDataImport = FindLastRow("REUTERS IMPORT") 'Loop for lookup

LastRowSummary = Findlastrowtable("SUMMARY DATA EXPORT VIEW") ' Loop for table.
For ColumnCounter = 10 To 33 ' Colums where summary table preside.

Exportdate = Sheets("SUMMARY DATA EXPORT VIEW").Cells(1, ColumnCounter).Value

For RowCounter = 81 To LastRowSummary ' This is th3 first cell to be evluated. For each colum

ExShipCount = 0
Exportdate = Sheets("SUMMARY DATA EXPORT VIEW").Cells(1, ColumnCounter).Value '
ExportCountry = Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, 1).Value 'For each cell being evaluated, we need to store the Export country in column 1 to be evaluated.
ImportCountry = Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, 2).Value 'For each cell being evaluated, we need to store the Import country in column 2 to be evaluated.

For DataImportCounter = 1 To LastRowDataImport ' Data import counter is delared as rows 1 to the outcome of the function FINDLASTROW

ExShipCount = WorksheetFunction.CountIfs(Range(Sheets("REUTERS IMPORT").Cells(DataImportCounter, 8)) = ExportCountry, Range(Sheets("REUTERS IMPORT").Cells(DataImportCounter, 10)) = ImportCountry, Range(Exportdate = Sheets("REUTERS IMPORT").Cells(DataImportCounter, 16)))


Next DataImportCounter
Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter) = ExShipCount ' Where to put the restuls. Colum counter and

Next RowCounter 'Once we Finish with the rows we move to the colums
Next ColumnCounter


End Sub

Function FindLastRow(ShtName) As Integer

For X = 81 To 50000
If Sheets(ShtName).Cells(X, 1) = "" Then
Exit For
End If
Next X

FindLastRow = X - 1

End Function

Function Findlastrowtable(ShtName) As Integer

For X = 81 To 50000
If Sheets(ShtName).Cells(X, 1).Value = "x" Then
Exit For
End If
Next X

Findlastrowtable = X - 1

End Function

 

[Mark_Annonyous]

Sub Countavol()


Dim DataImportCounter, RowCounter, ColumnCounter, ExShipCount, ImShipCount, LastRowDataImport, LastRow As Integer
Dim ExportCountry, ImportCountry As String
Dim Exportdate As Date

LastRowDataImport = FindLastRow("REUTERS IMPORT") 'Loop for lookup

LastRowSummary = Findlastrowtable("SUMMARY DATA EXPORT VIEW") ' Loop for table.
For ColumnCounter = 10 To 33 ' Colums where summary table preside.

Exportdate = Sheets("SUMMARY DATA EXPORT VIEW").Cells(1, ColumnCounter).Value

For RowCounter = 81 To LastRowSummary ' This is th3 first cell to be evluated. For each colum

ExShipCount = 0
Exportdate = Sheets("SUMMARY DATA EXPORT VIEW").Cells(1, ColumnCounter).Value '
ExportCountry = Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, 1).Value 'For each cell being evaluated, we need to store the Export country in column 1 to be evaluated.
ImportCountry = Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, 2).Value 'For each cell being evaluated, we need to store the Import country in column 2 to be evaluated.

For DataImportCounter = 1 To LastRowDataImport ' Data import counter is delared as rows 1 to the outcome of the function FINDLASTROW

ExShipCount = WorksheetFunction.CountIfs(Range(Sheets("REUTERS IMPORT").Cells(DataImportCounter, 8)) = ExportCountry, Range(Sheets("REUTERS IMPORT").Cells(DataImportCounter, 10)) = ImportCountry, Range(Exportdate = Sheets("REUTERS IMPORT").Cells(DataImportCounter, 16)))


Next DataImportCounter
Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter) = ExShipCount ' Where to put the restuls. Colum counter and

Next RowCounter 'Once we Finish with the rows we move to the colums
Next ColumnCounter


End Sub

Function FindLastRow(ShtName) As Integer

For X = 81 To 50000
If Sheets(ShtName).Cells(X, 1) = "" Then
Exit For
End If
Next X

FindLastRow = X - 1

End Function

Function Findlastrowtable(ShtName) As Integer

For X = 81 To 50000
If Sheets(ShtName).Cells(X, 1).Value = "x" Then
Exit For
End If
Next X

Findlastrowtable = X - 1

End Function

 

[jasonb75]

Without knowing what you're trying to do, isolating specific cause isn't always easy, the line ExShipCount = WorksheetFunction.... is one thing that stands out as wrong without looking at details.

Please note that these are observations, not criticisms, what you've done is much the same as anyone trying to learn unaided.

Your declarations, (Dim something) are not defined as anything useful, only the last one in the top row is actually Integer, the rest are Variant(s). Ideally, you should use Long instead of Integer. Integer doesn't have the capacity for bigger numbers used in modern applications.

The functions at the end could be trimmed down to 1 line each, looping is not needed to find the last row in the sheet.

 

[offthelip]

One of the main reasons that Vba is slow is the time taken to access the worksheet from VBa is a relatively long time.
To speed up vba the easiest way is to minimise the number of accesses to the worksheet. What is interesting is that the time taken to access a single cell on the worksheet in vba is almost identical as the time taken to access a large range if it is done in one action.

So instead of writing a loop which loops down a range copying one row at a time which will take along time if you have got 50000 rows it is much quicker to load the 50000 lines into a variant array ( one worksheet access), then copy the lines to a variant array and then write the array back to the worksheet, ( one worksheet access for each search that you are doing),

I have a simple rule for fast VBA: NEVER ACCESS THE WORKSHEET IN A LOOP.
I have rewritten part of your vba to show you how to use variant arrays instead of accessing the worksheet directly. This will make you macro much faster ( at least 1000 times faster) because you are accessing multiple cells on the worksheet in a double loop, so i am not surprised that took 30 minutes. I wasn't quite sure what second bit of the code was dong so I will let you rewrite that using variant arrays

Sub test()
Dim DataImportCounter, RowCounter, ColumnCounter, ExShipCount, ImShipCount, LastRowDataImport, LastRow As Integer
Dim ExportCountry, ImportCountry As String
Dim Exportdate As Date

LastRowDataImport = FindLastRow("REUTERS IMPORT") 'Loop for lookup

lastrowsummary = Findlastrowtable("SUMMARY DATA EXPORT VIEW") ' Loop for table.
With Worksheets("SUMMARY DATA EXPORT VIEW")
 alldata = Range(.Cells(1, 1), .Cells(lastrowsummary, 33))
 End With
For ColumnCounter = 10 To 33 ' Colums where summary table preside.

Exportdate = alldata(1, ColumnCounter)

For RowCounter = 81 To lastrowsummary ' This is th3 first cell to be evluated. For each colum

ExShipCount = 0
Exportdate = alldata(1, ColumnCounter) '
ExportCountry = alldata(RowCounter, 1) 'For each cell being evaluated, we need to store the Export country in column 1 to be evaluated.
ImportCountry = alldata(RowCounter, 2) 'For each cell being evaluated, we need to store the Import country in column 2 to be evaluated.

For DataImportCounter = 1 To LastRowDataImport ' Data import counter is delared as rows 1 to the outcome of the function FINDLASTROW

ExShipCount = WorksheetFunction.CountIfs(Range(Sheets("REUTERS IMPORT").Cells(DataImportCounter, 8)) = ExportCountry, Range(Sheets("REUTERS IMPORT").Cells(DataImportCounter, 10)) = ImportCountry, Range(Exportdate = Sheets("REUTERS IMPORT").Cells(DataImportCounter, 16)))


Next DataImportCounter
Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter) = ExShipCount ' Where to put the restuls. Colum counter and

Next RowCounter 'Once we Finish with the rows we move to the colums
Next ColumnCounter


End Sub

 

[Mark_Annonyous]

Thank you a lot, both of you for your answers. I really appreciate your friendly responses. I've written the original code below (I am trying to switch it to sumif because it takes too long, I lifted it form another source and applied to to my sheet. It works, but it takes roughly 40 mins to run. How am I able to adapt this to an array?

Again, thanks for your help

Sub lngvol()



Application.ScreenUpdating = False


Application.Calculation = xlCalculationManual

Dim DataImportCounter, RowCounter, ColumnCounter, ExShipCount, ImShipCount, LngSum, LastRowDataImport, LastRow As Integer
Dim ExportCountry, ImportCountry As String
Dim Exportdate As Date

LastRowDataImport = FindLastRow("REUTERS IMPORT")

LastRowSummary = FindLastRow2("SUMMARY DATA EXPORT VIEW")

For ColumnCounter = 10 To 33
     Exportdate = Sheets("SUMMARY DATA EXPORT VIEW").Cells(1, ColumnCounter).Value
    For RowCounter = 81 To LastRowSummary
        ExShipCount = 0
        ImShipCount = 0
        LngSum = 0
        ExportCountry = Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, 1).Value
        ImportCountry = Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, 2).Value
        For DataImportCounter = 1 To LastRowDataImport
            If Sheets("REUTERS IMPORT").Cells(DataImportCounter, 8) = ExportCountry Then
                If Sheets("REUTERS IMPORT").Cells(DataImportCounter, 10) = ImportCountry Then
                
                    If Exportdate = Sheets("REUTERS IMPORT").Cells(DataImportCounter, 16) Then
                       ExShipCount = ExShipCount + 1
                        LngSum = LngSum + Sheets("REUTERS IMPORT").Cells(DataImportCounter, 7).Value
                    End If
                End If
             End If
        Next DataImportCounter
        Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter) = ExShipCount
        Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter + 39) = LngSum
    Next RowCounter
Next ColumnCounter



Application.ScreenUpdating = True

Application.Calculation = xlCalculationAutomatic

End Sub

Function FindLastRow(ShtName) As Integer



For X = 81 To 50000
    If Sheets(ShtName).Cells(X, 1) = "" Then
        Exit For
    End If
Next X

FindLastRow = X - 1

End Function

Function FindLastRow2(ShtName) As Integer


For X = 81 To 50000
    If Sheets(ShtName).Cells(X, 1).Value = "x" Then
        Exit For
    End If
Next X

FindLastRow2 = X - 1

End Function

 

[Mark_Annonyous]

Just reading up non these arrays. I use them a lot in native excel{}, they do a lot to clean up spreadsheets deffinatly.

 

[offthelip]

Do be careful array formula on the worksheet has nothing in common with using arrays in vba. I have made an effort at recoding your vBA using variant arrays. The basic ideas is:
first load all the data from the worksheets into 2 variant arrays. "Alldata" and "Reutdata" We then defien two output variant arrays output1 and output2. The we execue your code having substituted the names of hte variant arrays everywhere you are addressing any cells. Then finally we write the two output arrays to the worksheet,. This code should only take a few seconds to run depending on what you last row is
Note the code is untested and I might have got some of the numbers ofr the addressing wrong. This is particularly difficult for the outputs because your outputs start in the middle of the worksheet.

Sub lngvol()



Application.ScreenUpdating = False


Application.Calculation = xlCalculationManual

Dim DataImportCounter, RowCounter, ColumnCounter, ExShipCount, ImShipCount, LngSum, LastRowDataImport, LastRow As Integer
Dim ExportCountry, ImportCountry As String
Dim Exportdate As Date

LastRowDataImport = FindLastRow("REUTERS IMPORT")
With Worksheets("REUTERS IMPORT")
 Reutdata = Range(.Cells(1, 1), .Cells(LastRowDataImport, 33))
 End With

LastRowSummary = FindLastRow2("SUMMARY DATA EXPORT VIEW")
With Worksheets("SUMMARY DATA EXPORT VIEW")
 alldata = Range(.Cells(1, 1), .Cells(LastRowSummary, 33))
 output1 = Range(.Cells(81, 10), .Cells(LastRowSummary, 33))
 output2 = Range(.Cells(81, 10 + 39), .Cells(LastRowSummary, 33 + 39))

 
For ColumnCounter = 10 To 33
     Exportdate = alldata(1, ColumnCounter)
    For RowCounter = 81 To LastRowSummary
        ExShipCount = 0
        ImShipCount = 0
        LngSum = 0
        ExportCountry = alldata(RowCounter, 1) 'For each cell being evaluated, we need to store the Export country in column 1 to be evaluated.
        ImportCountry = alldata(RowCounter, 2) 'For each cell being evaluated, we need to store the Import country in column 2 to be evaluated.
        For DataImportCounter = 1 To LastRowDataImport
            If Reutdata(DataImportCounter, 8) = ExportCountry Then
                If Reutdata(DataImportCounter, 10) = ImportCountry Then
                
                    If Exportdate = Reutdata(DataImportCounter, 16) Then
                       ExShipCount = ExShipCount + 1
                        LngSum = LngSum + Reutdata(DataImportCounter, 7)
                    End If
                End If
             End If
        Next DataImportCounter
        ' write the outputs to two variant arrays to avoid writing to the worksheet in a loop
        ' subtract 9 off the column and 80 off the row because the array starts at cell 81 ,10
        output1(RowCounter - 80, ColumnCounter - 9) = ExShipCount
'        Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter) = ExShipCount
         output2(RowCounter - 80, ColumnCounter - 9) = LngSum
 '       Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter + 39) = LngSum
    Next RowCounter
Next ColumnCounter
' Now write the oputpusto the worksheet in one go
  Range(.Cells(81, 10), .Cells(LastRowSummary, 33)) = output1
  Range(.Cells(81, 10 + 39), .Cells(LastRowSummary, 33 + 39)) = output2
 


End With
Application.ScreenUpdating = True

Application.Calculation = xlCalculationAutomatic

End Sub

Note because I use variant arrays all the time my code is usually very fast so I never need to bother about turning screenupdating off and turning the calculation to manual, because with the code I have written above it will make less than a millisecond difference in the time taken.

 

[TheRedCardinal]

I've nothing to add to the original question but wanted to second @offthelip and his comments on loading arrays into VBA.

One of the single best things I learned (on this website too) and well worth investing some time in "practice mode" learning these.

 

[Mark_Annonyous]

Offtheflip. Thankyou So much. It works a treat. I'm pretty sure i understand whats happening too. I'm going to watch a youtube tutorial tonight on it.

Again, thanks so much for the time you all took.

Appreciated.

 

[jasonb75]

Attempting to finish off what @offthelip started by cleaning up the redundant variables and declaring the rest correctly,

Option Explicit
Sub lngvol()

    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual

Dim LastRowDataImport As Long, LastRowSummary As Long, ColumnCounter As Long, RowCounter As Long, ExShipCount As Long, ImShipCount As Long
Dim DataImportCounter As Long
Dim ReutData, alldata, output1, output2
Dim Summary As Worksheet, ReutersImport As Worksheet
Dim ExportDate As Date, LngSum As Double, ExportCountry As String, ImportCountry As String

Set ReutersImport = Worksheets("REUTERS IMPORT")
Set Summary = Worksheets("SUMMARY DATA EXPORT VIEW")

With ReutersImport
    LastRowDataImport = .Cells(Rows.Count, 1).End(xlUp).Row ' This finds the last row without looping
    ReutData = Range(.Cells(1, 1), .Cells(LastRowDataImport, 33))
End With

With Summary
    LastRowSummary = .Cells(Rows.Count, 1).End(xlUp).Row ' This finds the last row without looping.
    alldata = Range(.Cells(1, 1), .Cells(LastRowSummary, 33))
    output1 = Range(.Cells(81, 10), .Cells(LastRowSummary, 33))
    output2 = Range(.Cells(81, 10 + 39), .Cells(LastRowSummary, 33 + 39))

 
    For ColumnCounter = 10 To 33
        ExportDate = alldata(1, ColumnCounter)
        
        For RowCounter = 81 To LastRowSummary
            ExShipCount = 0
            ImShipCount = 0
            LngSum = 0
            ExportCountry = alldata(RowCounter, 1) 'For each cell being evaluated, we need to store the Export country in column 1 to be evaluated.
            ImportCountry = alldata(RowCounter, 2) 'For each cell being evaluated, we need to store the Import country in column 2 to be evaluated.
            For DataImportCounter = 1 To LastRowDataImport
                If ReutData(DataImportCounter, 8) = ExportCountry Then
                    If ReutData(DataImportCounter, 10) = ImportCountry Then
                    
                        If ExportDate = ReutData(DataImportCounter, 16) Then
                            ExShipCount = ExShipCount + 1
                            LngSum = LngSum + ReutData(DataImportCounter, 7)
                        End If
                    End If
                End If
            Next DataImportCounter
                    ' write the outputs to two variant arrays to avoid writing to the worksheet in a loop
                    ' subtract 9 off the column and 80 off the row because the array starts at cell 81 ,10
                output1(RowCounter - 80, ColumnCounter - 9) = ExShipCount
                    ' Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter) = ExShipCount
                output2(RowCounter - 80, ColumnCounter - 9) = LngSum
                    ' Sheets("SUMMARY DATA EXPORT VIEW").Cells(RowCounter, ColumnCounter + 39) = LngSum
        Next RowCounter
    Next ColumnCounter
            ' Now write the oputpusto the worksheet in one go
        Range(.Cells(81, 10), .Cells(LastRowSummary, 33)) = output1
        Range(.Cells(81, 10 + 39), .Cells(LastRowSummary, 33 + 39)) = output2
End With

    Application.ScreenUpdating = True
    Application.Calculation = xlCalculationAutomatic

End Sub

I've also added in a couple of Find lines to eliminate the need for your functions looping to find the last row in each of the sheets.
There are a few other things that I would do differently, but that is more personal preference than a better way.