Speeding up my code

dpaton05

Well-known Member
Joined
Aug 14, 2018
Messages
2,352
Office Version
  1. 365
  2. 2016
Platform
  1. Windows
I have a table in my spreadsheet that goes from A-AP. It has visible columns up to column Q. From R to AP are hidden columns. The extra columns are hidden and they are just to work out certain information based on the data entered in the visible columns. For instance, I have a date that is entered in column A and in column Z I have this formula to work out the month the transaction needs to be recorded in based on the requirements of my workplace.
Excel Formula:
=IF(MONTH(A5)=6,"June",TEXT(DATE(YEAR(A5),IF(DAY(A5)<26,MONTH(A5),MONTH(A5)+1),1), "mmmm"))

In column AA
Excel Formula:
=TEXT(DATE(YEAR(A5),IF(DAY(A5)<26,MONTH(A5),MONTH(A5)+1),1), "yyyy")


That is just 2 of the columns. The table ends at the column AP with other formulas that derive their data from the visible cells

I have a copy procedure that looks at every row in the table and copies information from the row to 2 separate documents. These separate documents are financial year documents, broken up into monthly sheets. Parts of each row in the table are copied to monthly sheets in the 2 documents. For about 100 rows entered in the table, in takes around 5 minutes to run the code. This doesn't seem like I have developed it in the most efficient manner and I am sure it can be executed much faster.

I am still learning vba, so when I had something working, I didn't want to break it again so I left it. Other people helped me with most of it too so I couldn't change parts due to not knowing how.

I am not sure if it can be run faster but could someone look at my code and give a few ideas on how I could speed it up please? I have been told that the more you need to interact with the worksheet, the more it slows down. Maybe I should try and move alot of the additional, hidden columns into vba so it doesn't need to interact with the worksheet so much?

Here is my copy procedure.
You may notice that there is code relating to an hours register file that is commented out. I do not need it anymore at the moment but I may need it later.

VBA Code:
Sub cmdCopy()
'On Error GoTo ErrorMsg
    Dim wsDst As Worksheet, wsHours As Worksheet, wsTrack As Worksheet, worker As String, wsSrc As Worksheet, tblrow As ListRow
    Dim Combo As String, sht As Worksheet, tbl As ListObject
    Dim LastRow As Long, DocYearName As String, Site As String, lr As Long, HoursRow As Long
    Dim RowColor As Long, w As Window, r As Long, HoursRegister As String, ReportTracking As String
        Application.ScreenUpdating = False
       
    'assign values to variables
    Set tbl = ThisWorkbook.Worksheets("Costing_tool").ListObjects("tblCosting")
    Set sht = ThisWorkbook.Worksheets("Costing_tool")
    Site = ThisWorkbook.Worksheets("Start_here").Range("H9").Value
    'Check if each row has a date, service and requesting organisation
    For Each tblrow In tbl.ListRows
        If tblrow.Range.Cells(1, 1).Value = "" Or tblrow.Range.Cells(1, 5).Value = "" Or tblrow.Range.Cells(1, 6).Value = "" Then
            MsgBox "The Date, Service or Requesting Organisation has not been entered for every record in the table"
            Exit Sub
        End If
    Next tblrow
    For Each tblrow In tbl.ListRows
        'Define combo as the month to be recorded in
        Combo = tblrow.Range.Cells(1, 26).Value
        'If column 8 for the row is blank...
        If Not tblrow.Range(1, 8).Value = "" Then
            'worker variable is defined as the value in column 8 of the row
            worker = tblrow.Range.Cells(1, 8).Value
        Else
            'otherwise, "not allocated" is assigned to the worker variable.
            'this is used in the hours register to identify which sheet to place the hours in
            worker = "Not allocated"
        End If
        'defines HoursRegister as the hours register filename that is stored in column 38 for the row
'HoursRegister = tblrow.Range.Cells(1, 38)
        'defines ReportTracking as the report tracking filename that is stored in column 39 for the row
        ReportTracking = tblrow.Range.Cells(1, 39)
            Select Case Site
                Case "Wes"
                    Select Case tblrow.Range.Cells(1, 6).Value
                        Case "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
                            DocYearName = tblrow.Range.Cells(1, 37).Value
                        Case Else
                            DocYearName = tblrow.Range.Cells(1, 36).Value
                    End Select
                Case "Riv"
                    Select Case tblrow.Range.Cells(1, 6).Value
                        Case "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
                            DocYearName = tblrow.Range.Cells(1, 42).Value
                        Case Else
                            DocYearName = tblrow.Range.Cells(1, 36).Value
                    End Select

            End Select
        If Not isFileOpen(DocYearName & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Work Allocation Sheets" & "\" & Site & "\" & DocYearName & ".xlsm"
'If Not isFileOpen(HoursRegister & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Hours Register" & "\" & Site & "\" & HoursRegister & ".xlsm"
        If Not isFileOpen(ReportTracking & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Report Tracking" & "\" & Site & "\" & ReportTracking & ".xlsm"
'Set wsHours = Workbooks(HoursRegister).Worksheets(worker)
        Set wsDst = Workbooks(DocYearName).Worksheets(Combo)
        Set wsTrack = Workbooks(ReportTracking).Worksheets(Combo)
        lr = wsDst.Cells.Find("*", , xlValues, , xlRows, xlPrevious).Row
       
        'Copy the pricing cells from the quoting tool to the allocation sheet for use in calculating late cancels
        Workbooks(DocYearName).Worksheets("sheet2").Range("A4:E12").Value = Data.Range("A4:E12").Value
       
'With wsHours
      'this copies the date column in the tblCosting
    'HoursRow = .Range("A" & Rows.Count).End(xlUp).Row
    'tblrow.Range(, 1).Copy
    'this pastes it into column A of hours register file
    '.Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
      'this copies the YP name column in the tblCosting
    'tblrow.Range(, 4).Copy
    'this pastes it into column B of hours register file
    '.Range("B" & HoursRow).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
       'this copies the YP name column in the tblCosting
    'tblrow.Range(, 3).Copy
    'this pastes it into column A of hours register file
    '.Range("C" & HoursRow).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
       'this copies the hours column in the tblCosting
    'tblrow.Range(, 9).Copy
    'this pastes it into column A of hours register file
    '.Range("D" & HoursRow).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
'End With
        With wsTrack
              'this copies the date column in the tblCosting
            tblrow.Range(, 1).Copy
            'this pastes it into column A of hours register file
            .Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
            'this copies the YP name column in the tblCosting
            tblrow.Range(, 4).Copy
            'this pastes it into column B of the report tracking file
            .Range("A" & Rows.Count).End(xlUp).Offset(, 1).PasteSpecial xlPasteFormulasAndNumberFormats
               'this copies the YP name column in the tblCosting
            tblrow.Range(, 5).Copy
            'this pastes it into column A of hours register file
            .Range("A" & Rows.Count).End(xlUp).Offset(, 2).PasteSpecial xlPasteFormulasAndNumberFormats
        End With
        With wsDst
                'This sets column width of request number column so it can be read and is not xxxxx
                .Columns("C:C").ColumnWidth = 8
               
                'This copies the first 7 columns, i.e. A:G, of the current row of the table to column A in the destination sheet.
                tblrow.Range.Resize(, 7).Copy
                'This pastes in the figures in the first 7 columns starting in column A
                .Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
                'This copies the first 7 columns, i.e. A:G, of the current row of the table to column A in the destination sheet.
                tblrow.Range(, 10).Copy
               
                'This pastes in the figures in the first 7 columns starting in column A
                .Range("H" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
               
                'Overwrites the numbers pasted to column I with a formula
                .Range("I" & Rows.Count).End(xlUp).Offset(1).Formula = "=IF(RC[-4]=""Activities"",0,RC[-1]*0.1)"
                'Overwrites the numbers pasted to column L with a formula
                .Range("J" & Rows.Count).End(xlUp).Offset(1).Formula = "=RC[-1]+RC[-2]"
                'Adds currency formatting to total ex gst column
                .Columns(8).NumberFormat = "$#,##0.00"
                'Adds Australian date format to date column
                '.Range("A:A").NumberFormat = "dd/mm/yyyy"
   
    
                'sort procedure copied from vba
                wsDst.Sort.SortFields.Clear
                wsDst.Sort.SortFields.Add Key:=Range("A4:A" & lr), _
                    SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
                        With Workbooks(DocYearName).Worksheets(Combo).Sort
                            'set range to sort of A3 to AO
                            .SetRange Range("A3:AO" & lr)
                            .header = xlYes
                            .MatchCase = False
                            .Orientation = xlTopToBottom
                            .SortMethod = xlPinYin
                            .Apply
                        End With
        End With
    Next tblrow
    Application.CutCopyMode = False
    Application.ScreenUpdating = True
Exit Sub

'ErrorMsg:
'    Select Case Err.Number
'        Case 53
'            MsgBox "Enable macros needs to be selected"
'    End Select
End Sub
 
Last edited by a moderator:
I think some of my lines of code don't line up with the comments as I have changed the code by finding code online or someone wrote updated code for me and I didn't change the comments.

The wsDst part is a bit confusing, sorry , I forgot to explain it. In the outer columns in tblCosting, I have the name of those additional spreadsheets stored, the ones I transfer the data to. I am not at the computer at the moment so I will try and send an explanation tomorrow as I am about to go to sleep.

Thank you for your continued support
 
Upvote 0

Excel Facts

Show numbers in thousands?
Use a custom number format of #,##0,K. Each comma after the final 0 will divide the displayed number by another thousand
This is tblCosting with one entry

CSS_quoting_tool_29.5.xlsm
ABCDEFGHIJKLMNOPQRSTUVWXYZAAABACADAEAFAGAHAIAJAKALAMANAOAP
4DatePurchase order #Quote Ref #Child NameServiceRequesting OrganisationCaseworker NameAllocated toWait Time/HrsPrice ex. GSTGSTPrice inc. GSTDate report receivedDate report sentAllocated byReport sent byYouth worker/CarerStaff #KmsAdd. StopsDay Rateextra pickup$ 4 extra hrs$ 4 kmsMax PayMonthYearCombinationDocument nameNew doc name10% increaseActivitiesFirst YearSecond YearFinancial YearExternalASInternalASHoursRegisterReportTrackingColumn1Column2Column3
507/07/2020BobActivitiesSome organisationMeYou2$56.00$0.00$56.00July20202021 - 2022202020212020 - 20212020 - 2021 Western CSS Work Allocation Sheet2020 - 2021 Western Internal Work Allocation Sheet2020 - 2021 Western Hours Register2020 - 2021 Western Report Tracking2020 - 2021 Riv Internal Work Allocation Sheet
Costing_tool
Cell Formulas
RangeFormula
K5K5=IF(E5="Activities",0,[Price ex. GST]*0.1)
L5L5=IF(E5="Activities",[@[Price ex. GST]],[GST]+[Price ex. GST])
Z5Z5=IF(MONTH(A5)=6,"June",TEXT(DATE(YEAR(A5),IF(DAY(A5)<26,MONTH(A5),MONTH(A5)+1),1), "mmmm"))
AA5AA5=TEXT(DATE(YEAR(A5),IF(DAY(A5)<26,MONTH(A5),MONTH(A5)+1),1), "yyyy")
AD5AD5=CONCATENATE(AA5+1," - ",AA5+2)
AG5AG5=IF(MONTH(A5)<7,YEAR(A5)-1,YEAR(A5))
AH5AH5=IF(MONTH(A5)<7,YEAR(A5),YEAR(A5)+1)
AI5AI5=CONCATENATE(AG5," - ",AH5)
AJ5AJ5=CONCATENATE(AI5," ",Start_here!$H$9," CSS Work Allocation Sheet")
AK5AK5=CONCATENATE(AI5," ",Start_here!$H$9," Internal Work Allocation Sheet")
AL5AL5=CONCATENATE(AI5," ",Start_here!$H$9," Hours Register")
AM5AM5=CONCATENATE(AI5," ",Start_here!$H$9," Report Tracking")
AP5AP5=CONCATENATE(AI5," ","Riv Internal Work Allocation Sheet")
Cells with Data Validation
CellAllowCriteria
D5,G5:I5,A5Any value
G5:I5Any value


This is used for generating quotes and H9 on start_here has the options Western or Riv as the organisation that generates the quote. They are stored in the following format



I don't have the file with me at the moment so I can't quite remember the file structure but I think this is it

Each new line is a new folder level, the first line is a folder in the same folder as the file that runs this code
Quote tools
Work Allocation sheets
Western, Riv


What wsDst is doing is copying variouis columns from tblCosting into these allocation sheets that are used as the final resting place for the data. Only some of the rows are copied and other rows are used to identify the filename of the relevant file that the entry needs to be put in.



This is what the allocation sheets look like. They are all the same, just named different. This is the June sheet but there are sheets for every month of the year.
CSS_quoting_tool_29.5.xlsm
ABCDEFGHIJKLMNOPQRSTUVWXYZAAABACADAEAFAGAHAIAJAKALAMANAOAP
4DatePurchase order #Quote Ref #Client NameServiceRequesting Organisationworker NameAllocated toWait Time/HrsPrice ex. GSTGSTPrice inc. GSTDate report receivedDate report sentAllocated byReport sent byWorkerStaff #KmsAdd. StopsDay Rateextra pickup$ 4 extra hrs$ 4 kmsMax PayMonthYearCombinationDocument nameNew doc name10% increaseActivitiesFirst YearSecond YearFinancial YearExternalASInternalASHoursRegisterReportTrackingColumn1Column2Column3
507/07/2020BobActivitiesSome organisationMeYou2$56.00$0.00$56.00July20202021 - 2022202020212020 - 20212020 - 2021 Western CSS Work Allocation Sheet2020 - 2021 Western Internal Work Allocation Sheet2020 - 2021 Western Hours Register2020 - 2021 Western Report Tracking2020 - 2021 Riv Internal Work Allocation Sheet
Costing_tool
Cell Formulas
RangeFormula
K5K5=IF(E5="Activities",0,[Price ex. GST]*0.1)
L5L5=IF(E5="Activities",[@[Price ex. GST]],[GST]+[Price ex. GST])
Z5Z5=IF(MONTH(A5)=6,"June",TEXT(DATE(YEAR(A5),IF(DAY(A5)<26,MONTH(A5),MONTH(A5)+1),1), "mmmm"))
AA5AA5=TEXT(DATE(YEAR(A5),IF(DAY(A5)<26,MONTH(A5),MONTH(A5)+1),1), "yyyy")
AD5AD5=CONCATENATE(AA5+1," - ",AA5+2)
AG5AG5=IF(MONTH(A5)<7,YEAR(A5)-1,YEAR(A5))
AH5AH5=IF(MONTH(A5)<7,YEAR(A5),YEAR(A5)+1)
AI5AI5=CONCATENATE(AG5," - ",AH5)
AJ5AJ5=CONCATENATE(AI5," ",Start_here!$H$9," CSS Work Allocation Sheet")
AK5AK5=CONCATENATE(AI5," ",Start_here!$H$9," Internal Work Allocation Sheet")
AL5AL5=CONCATENATE(AI5," ",Start_here!$H$9," Hours Register")
AM5AM5=CONCATENATE(AI5," ",Start_here!$H$9," Report Tracking")
AP5AP5=CONCATENATE(AI5," ","Riv Internal Work Allocation Sheet")
Cells with Data Validation
CellAllowCriteria
D5,G5:I5,A5Any value
G5:I5Any value


I am not sure what else information you need. Please ask me some questions if you need to clarifiy anything.

Thanks.
 
Upvote 0
"What wsDst is doing is copying variouis columns from tblCosting into these allocation sheets that are used as the final resting place for the data. Only some of the rows are copied and other rows are used to identify the filename of the relevant file that the entry needs to be put in."
This was the only bit of information that is useful to me, now you need to specify, which rows and which columns are copied to which rows columns in which files
 
Upvote 0
The code I have correctly identifies the file to put each row in. I will try and explain my code.


VBA Code:
Site = ThisWorkbook.Worksheets("Start_here").Range("H9").Value
This is assigning the location of the organisation completing the transaction to the Site variable. This is selected from a drop down box on the Start_here sheet. The drop down box has the options Western and Riv. Start_here is the first sheet that is displayed when you open the file.

Every row in tblCosting is a new quote with a date, service type,


Back to your question
  • If Western is selected on the Start_here sheet
VBA Code:
Select Case Site
Case "Western"

    • If one of these organisations is then entered in column 6 of tblCosting "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
VBA Code:
                   Select Case tblrow.Range.Cells(1, 6).Value
                        Case "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
    • the value in column 37 for the row contains the correct, destination filename. This filename is then assigned to the DocYearName variable. Please note, DocYearName does not have the extension in the filename
VBA Code:
                           DocYearName = tblrow.Range.Cells(1, 37).Value


  • If The organisation entered in tblCosting is something different from "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
    • the value in column 36 has the correct filename. This filename is then assigned to the DocYearName variable
VBA Code:
                            Case Else
                                DocYearName = tblrow.Range.Cells(1, 36).Value

The rest of this part of the code has the same format, just with different column numbers

  • If Riv is selected on the Start_here sheet
    • If one of these organisations is entered in tblCosting "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
      • the value in column 42 for the row contains the correct, destination filename. This filename is then assigned to the DocYearName variable.
    • If The organisation entered in tblCosting is something different from "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
      • the value in column 36 has the correct filename. This filename is then assigned to the DocYearName variable

This line describes the directory structure
VBA Code:
If Not isFileOpen(DocYearName & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Work Allocation Sheets" & "\" & Site & "\" & DocYearName & ".xlsm"
In the Work Allocation Sheets folder, is 2 folders, Western and Riv. The allocation sheets are stored in the relevant folders for the site.


VBA Code:
        Combo = tblrow.Range.Cells(1, 26).Value
This code assigns the month stored in column 26 to the combo variable. This is the correct month or sheet within the allocation sheet, where the row needs to go. Column 26 has the following function
Excel Formula:
=IF(MONTH(RC[-25])=6,"June",TEXT(DATE(YEAR(RC[-25]),IF(DAY(RC[-25])<26,MONTH(RC[-25]),MONTH(RC[-25])+1),1), "mmmm"))



VBA Code:
        With wsTrack
            'this copies the date column in the tblCosting
            tblrow.Range(, 1).Copy
            'this pastes it into column A of the report tracking file
            .Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteValues
            'this copies the YP name column in the tblCosting
            tblrow.Range(, 4).Copy
            'this pastes it into column B of the report tracking file
            .Range("A" & Rows.Count).End(xlUp).Offset(, 1).PasteSpecial xlPasteValues
            'this copies the service type in column 5 of tblCosting
            tblrow.Range(, 5).Copy
            'this pastes it into column C of the Report tracking file
            .Range("A" & Rows.Count).End(xlUp).Offset(, 2).PasteSpecial xlPasteValues
        End With
Which column to which column wsTrack (the report tracking file), for each row
  • The date in tblCosting is in column 1
    • It is copied to the report tracking file in column 1
  • The name in column 4 of tblCosting
    • Is copied to the report tracking file in column 2
  • The service in column 5 of tblCosting
    • Is copied to the report tracking file in column 3

explanation of the with wsDst block


VBA Code:
Set wsDst = Workbooks(DocYearName).Worksheets(Combo)
This sets wsDst as the worksheet identified by Combo that is within the workbook identified by DocYearName

tblCosting and the allocation sheets are very similar so this line of code copies the first 7 columns from tblCosting to the allocation first 7 columns of the allocation sheets
VBA Code:
                tblrow.Range.Resize(, 7).Copy
                .Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteValues


This code copies the price ex. gst from column 10 to column 8 on the allocation sheet. Note that I can't include it in the above code by making the region bigger to copy as I don't want columns 8 and 9 in tblCosting included.
VBA Code:
                tblrow.Range(, 10).Copy
                .Range("H" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteValues


This line enters a formula in the allocation sheet in column 9 for the row
VBA Code:
.Range("I" & Rows.Count).End(xlUp).Offset(1).Formula = "=IF(RC[-4]=""Activities"",0,RC[-1]*0.1)"


This line enters a formula in the allocation sheet in column 10 for the row
VBA Code:
.Range("J" & Rows.Count).End(xlUp).Offset(1).Formula = "=RC[-1]+RC[-2]"

VBA Code:
ReportTracking = tblrow.Range.Cells(1, 39)
This code assigns the report tracking filename stored in column 39 to the ReportTracking variable. This does not have the extension.



This block is just a few formatting changed to the allocation sheet that are performed

VBA Code:
    With wsDst
        'This sets column width of request number column so it can be read and is not xxxxx
        .Columns("C:C").ColumnWidth = 8
        'Adds currency formatting to total ex gst column
        .Columns(8).NumberFormat = "$#,##0.00"
    End With



I think I have included everything, let me know if there is anything I have missed. Reading through my code, I realised that I didn't update some comments when I updated the code so no wonder you had difficulty following it as they didn't say what was meant to happen.

I'm sorry for the extra long explanation.
Thanks again for your help :)
 
Upvote 0
Your description is describing what your workbook and your system is doing and it obviously fairly complex, however if I have understood what you macro is doing ( mainly by looking at your code) I think what is it doing is very simple: ( your lengthy description doesn’t really describe what MACRO is doing, far too much unnecessary detail)

For each row in the tblcosting table you are copying columns 1 ,4 and 5 to the next free line in a workbook where the name for the workbook is given column 39 of the table

Then for each row in the tblcosting table you are copying columns 1 to 7 to columns 1 to 7 of a workbook where the name of it is given by some logic which chooses between columns 36,37 and 42 in the table.

Column 10 is copied to column 8 in the same workbook

Equations are put into columns 9 and 10 in the same workbook

You then sort this workbook, before going onto the next row in the tblcosting ttable.

I have changed your code to speed it up a bit by loading the input table into a variant array and using for every case where you were accessing your input table. This eliminates about 20 accesses to the worksheet on every iteration. This may be sufficient improvement but there is lots more you can do if necessary.

The way you have structured your code makes it very slow. You could be setting wsDst and wsTrack to different workbooks every iteration. This is going to be slow. Also you are sorting the wsDst workbook EVERY iteration. This is going to be slow as well.

A much faster way of structuring your code is pick up the data from your table, look at the first row, open the two workbooks as necessary, process the first row, now go through the rest of the table process all the rows that use those two workbooks, make a check mark when a row is processed. (easily done in software) Now you back to the top of the table start on the second row, check if it has already been processed if not do that as per the first row. Etc etc,

If process it like this you can then do the sort on wsdst just once at the end because you know you have processed the whole table by then

How much you save here depends on how much commonality there is between each row and whether many of the use both those two. Because if that number if very small it could be faster to do all copying for Wstrack for the whole table, and then do all the copying for wsDst. Even doing it a separate subroutines

So one question is how many report tracking files are there and how Docyearname files are there?

Another way of speeding up your code is to use variant arrays to OUTPUT the information to the worksheet, this will save nearly as much as I have saved by changing the input since there are 13 accesses to the worksheets for outputs every iteration

Hopefully the code I have written will work but I haven’t been able to test it, and I can’t even compile it because there are too many undefines. I will post the code separately
 
Upvote 0
code:
VBA Code:
Sub cmdCopy()
'On Error GoTo ErrorMsg
    Dim wsDst As Worksheet, wsHours As Worksheet, wsTrack As Worksheet, worker As String, wsSrc As Worksheet, tblrow As ListRow
    Dim Combo As String, sht As Worksheet, tbl As ListObject
    Dim LastRow As Long, DocYearName As String, Site As String, lr As Long, HoursRow As Long
    Dim RowColor As Long, w As Window, r As Long, HoursRegister As String, ReportTracking As String
    Dim inarr As Variant
    
        Application.ScreenUpdating = False
      
    'assign values to variables
    Set tbl = ThisWorkbook.Worksheets("Costing_tool").ListObjects("tblCosting")
    Set sht = ThisWorkbook.Worksheets("Costing_tool")
    Site = ThisWorkbook.Worksheets("Start_here").Range("H9").Value
    'Check if each row has a date, service and requesting organisation
inarr = ThisWorkbook.Worksheets("Costing_tool").ListObjects("tblCosting").DataBodyRange.Value

For i = 1 To UBound(inarr, 1)
If inarr(i, 1) = "" Or inarr(i, 5) = "" Or inarr(i, 6) = "" Then
            MsgBox "The Date, Service or Requesting Organisation has not been entered for every record in the table"
            Exit Sub
  End If
Next i
'For Each tblrow In tbl.ListRows
For i = 1 To UBound(inarr, 1)
       
        'Define combo as the month to be recorded in
'        Combo = tblrow.Range.Cells(1, 26).Value
        Combo = inarr(i, 26)
        'If column 8 for the row is blank...
'        If Not tblrow.Range(1, 8).Value = "" Then
        If Not inarr(i, 8) = "" Then
            'worker variable is defined as the value in column 8 of the row
            'worker = tblrow.Range.Cells(1, 8).Value
            worker = inarr(i, 8)
        Else
            'otherwise, "not allocated" is assigned to the worker variable.
            'this is used in the hours register to identify which sheet to place the hours in
            worker = "Not allocated"
        End If
        'defines HoursRegister as the hours register filename that is stored in column 38 for the row
'HoursRegister = tblrow.Range.Cells(1, 38)
        'defines ReportTracking as the report tracking filename that is stored in column 39 for the row
       
        'ReportTracking = tblrow.Range.Cells(1, 39)
        ReportTracking = inarr(i, 39)
            Select Case Site
                Case "Wes"
                    Select Case inarr(i, 6)
'                    Select Case tblrow.Range.Cells(1, 6).Value
                        Case "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
                            'DocYearName = tblrow.Range.Cells(1, 37).Value
                            DocYearName = inarr(i, 37)
                        Case Else
                            'DocYearName = tblrow.Range.Cells(1, 36).Value
                            DocYearName = inarr(i, 36)
                    End Select
                Case "Riv"
                    'Select Case tblrow.Range.Cells(1, 6).Value
                    Select Case inarr(i, 6).Value
                        Case "Ang Wes", "Ang Wa", "Ang Al", "Ang SC", "Yiri"
                            DocYearName = inarr(i, 42)
                        Case Else
                            DocYearName = inarr(i, 36)
                    End Select

            End Select
        If Not isfileopen(DocYearName & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Work Allocation Sheets" & "\" & Site & "\" & DocYearName & ".xlsm"
'If Not isFileOpen(HoursRegister & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Hours Register" & "\" & Site & "\" & HoursRegister & ".xlsm"
        If Not isfileopen(ReportTracking & ".xlsm") Then Workbooks.Open ThisWorkbook.Path & "\" & "Report Tracking" & "\" & Site & "\" & ReportTracking & ".xlsm"
'Set wsHours = Workbooks(HoursRegister).Worksheets(worker)
        Set wsDst = Workbooks(DocYearName).Worksheets(Combo)
        Set wsTrack = Workbooks(ReportTracking).Worksheets(Combo)
        lr = wsDst.Cells.Find("*", , xlValues, , xlRows, xlPrevious).Row
      
        'Copy the pricing cells from the quoting tool to the allocation sheet for use in calculating late cancels
        Workbooks(DocYearName).Worksheets("sheet2").Range("A4:E12").Value = Data.Range("A4:E12").Value ' NOTE DATA DOESN'T SEEM TO BE DEFINED!!
      
'With wsHours
      'this copies the date column in the tblCosting
    'HoursRow = .Range("A" & Rows.Count).End(xlUp).Row
    'tblrow.Range(, 1).Copy
    'this pastes it into column A of hours register file
    '.Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
      'this copies the YP name column in the tblCosting
    'tblrow.Range(, 4).Copy
    'this pastes it into column B of hours register file
    '.Range("B" & HoursRow).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
       'this copies the YP name column in the tblCosting
    'tblrow.Range(, 3).Copy
    'this pastes it into column A of hours register file
    '.Range("C" & HoursRow).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
       'this copies the hours column in the tblCosting
    'tblrow.Range(, 9).Copy
    'this pastes it into column A of hours register file
    '.Range("D" & HoursRow).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
'End With
        With wsTrack
              'this copies the date column in the tblCosting
           ' tblrow.Range(, 1).Copy
            'this pastes it into column A of hours register file
            '.Range("A" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
            .Range("A" & Rows.Count).End(xlUp).Offset(1) = inarr(i, 1)
            'this copies the YP name column in the tblCosting
'            tblrow.Range(, 4).Copy
            'this pastes it into column B of the report tracking file
            .Range("A" & Rows.Count).End(xlUp).Offset(, 1) = inarr(i, 4)
               'this copies the YP name column in the tblCosting
'            tblrow.Range(, 5).Copy
            'this pastes it into column A of hours register file
            .Range("A" & Rows.Count).End(xlUp).Offset(, 2) = inarr(i, 5)
        End With
        With wsDst
          ' I am not sure what you are trying to do here but it can be improved
                'This sets column width of request number column so it can be read and is not xxxxx
              '  .Columns("C:C").ColumnWidth = 8 do this once at the end!!!
              
                'This copies the first 7 columns, i.e. A:G, of the current row of the table to column A in the destination sheet.
                'tblrow.Range.Resize(, 7).Copy
                'This pastes in the figures in the first 7 columns starting in column A
                
                For kk = 1 To 7
                .Range(Cells(lr, kk), Cells(lr, kk)) = inarr(i, kk)
                Next kk
                ' this copies column 10 to column 8
                .Range(Cells(lr, 8), Cells(lr, 8)) = inarr(i, 10)
                
                'This copies the first 7 columns, i.e. A:G, of the current row of the table to column A in the destination sheet.
                ' the comment doesn't seem t otie up wit the code here what are you doing??
                
'                tblrow.Range(, 10).Copy
              
                'This pastes in the figures in the first 7 columns starting in column A
 '               .Range("H" & Rows.Count).End(xlUp).Offset(1).PasteSpecial xlPasteFormulasAndNumberFormats
              
                'Overwrites the numbers pasted to column I with a formula
               ' .Range("I" & Rows.Count).End(xlUp).Offset(1).Formula = "=IF(RC[-4]=""Activities"",0,RC[-1]*0.1)"
                .Range(Cells(lr, 9), Cells(lr, 9)).Formula = "=IF(RC[-4]=""Activities"",0,RC[-1]*0.1)"

                'Overwrites the numbers pasted to column L with a formula
'                .Range("J" & Rows.Count).End(xlUp).Offset(1).Formula = "=RC[-1]+RC[-2]"
                .Range(Cells(lr, 10), Cells(lr, 10)).Formula = "=RC[-1]+RC[-2]"

                'Adds currency formatting to total ex gst column
 '               .Columns(8).NumberFormat = "$#,##0.00" do this at the end
                'Adds Australian date format to date column
                '.Range("A:A").NumberFormat = "dd/mm/yyyy"
  
   
                'sort procedure copied from vba
                'DO NOT DO THIS SORT ON EVERY ITERATION IT WILL BE MAJOR CAUSE OF YOUR TIME PROBLEM
                wsDst.Sort.SortFields.Clear
                wsDst.Sort.SortFields.Add Key:=Range("A4:A" & lr), _
                    SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
                        With Workbooks(DocYearName).Worksheets(Combo).Sort
                            'set range to sort of A3 to AO
                            .SetRange Range("A3:AO" & lr)
                            .Header = xlYes
                            .MatchCase = False
                            .Orientation = xlTopToBottom
                            .SortMethod = xlPinYin
                            .Apply
                        End With
        End With
'    Next tblrow
    Next i
    Application.CutCopyMode = False
    Application.ScreenUpdating = True
Exit Sub

'ErrorMsg:
'    Select Case Err.Number
'        Case 53
'            MsgBox "Enable macros needs to be selected"
'    End Select
End Sub
 
Upvote 0
Thanks again for your reply and thanks for the code you included. I am not able to test the code until I get back to work on Monday but I can answer several questions about it.

There will mainly be 1 DocYearName and 1 report tracking workbook but there are instances where there could be 2 or more but these are not as common. Quotes are generated for generally the current financial year but cases exist where quotes will need to be done for other financial years. I have a table, and after inputs, generates a quote for services to be performed. This quote is then copied to tblCosting for more information regarding the quote to be entered. There could be x number of quotes at any time in tblCosting. From here the quote is transferred to yearly allocation sheets, that the monthly sheet where each quote row is transferred is defined by the variable DocYearName. I have other processes that work well for removing the quote if it is cancelled for any reason.


The DocYearName and report tracking workbooks are financial year documents and if the date on one row in tblCosting is in the 20-21 financial year, it will go into the 20-21 document of both instances of the above mentioned documents.

You say that there is more you can do to speed it up and that sounds great. I would love to make this as fast as possible as this would impress my supervisor so much. I have already turned a process of 20+ minutes into a 5+ minute process. If I made it even faster, he would love that.

I am only new to vba so I didn't know there were different ways to structure code and that some are faster then others.


I like the sound of this part, could you tell me more about this please?
A much faster way of structuring your code is pick up the data from your table, look at the first row, open the two workbooks as necessary, process the first row, now go through the rest of the table process all the rows that use those two workbooks, make a check mark when a row is processed. (easily done in software) Now you back to the top of the table start on the second row, check if it has already been processed if not do that as per the first row. Etc etc,


What do you mean by this sentence?
How much you save here depends on how much commonality there is between each row and whether many of the use both those two.


Could you also tell me how I could use this to speed up the process, please?
Another way of speeding up your code is to use variant arrays to OUTPUT the information to the worksheet, this will save nearly as much as I have saved by changing the input since there are 13 accesses to the worksheets for outputs every iteration


Thanks
 
Upvote 0
My statement saying " How much you save here depends on the commonality there is between rows" is explained by your statement that you normally only have one "docyearname" and one "reporttracking" workbooks , This means there is a lot commonality between the different rows in your table, since most rows are identical. This means you will NOT save much time by going for the method of restructuring I was suggesting. In fact it would take longer in the case that they are all using the same two files. However what it does mean is that you can very easily save a lot of time by moving the sorting of the WsDst outside the loop. Since you may get more than one Wsdst files, the way I would do it is to keep a track of when you open a new Wsdst file and then at the end after the main loop set up another loop to select each wsdsts files ( they will already be open) and do the sort. This will save a lot of time, .
 
Upvote 0
Thank you for that. Could you please give me a few ideas in how I could code that?
 
Upvote 0
Actually, I will have a go myself and let you know if I run into problems.
 
Upvote 0

Forum statistics

Threads
1,214,911
Messages
6,122,194
Members
449,072
Latest member
DW Draft

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top