Excel Slow performance

**SamT** · 09-11-2017, 05:40 AM

Paul, I am a lazy typist.

 Dim WsF as Object
Set WsF = Applcation.WorksheetFunction
'
'
.Cells(14).Value = "WK" & Wsf.WeekNum(.Cells(2).Value)

For daily use

Sub AddData(Optional NewData As Range)
'
'
If Not NewData is Nothing Then NewData.Select
    If Not TypeOf Selection Is Range Then Exit Sub 
'
'

Public Sub Run_AddData_On_NewData()
Dim NewData As Range
Set NewData = AppropriateColumn.Find(First Formula) 'Edit to Code
Set NewData = Range(NewData, NewData.End.(xlDown))

AddData NewData

End Sub

**Paul_Hossler** · 09-11-2017, 05:48 AM

Originally Posted by offthelip

Paul

As I said earlier the way to really speed up the code is to avoid accessing the worksheet in a loop. You are still doing multiple accesses to the worksheet in a loop, all you have really done is reduce the loop to what has changed by selecting the rows that have changed. Unfortuantely I don't have time to rework this using variant arrays at the moment, but I am sure it would be much faster.

Yes, but as I said doing it in a daily batch would not take a lot of wall clock time

The first run of 100K+ rows might take some time of course, but I opted for the simplest, most easily understood and most easily maintained approach for the OP to self-maintain so I made it 'Formula-like'

I deliberately avoided a complicated, sophisticated VBA array approach since (while some members here could maintain it) I wanted the OP to have something simple that worked, even if not the highest performance

Personal Opinion: 99.9999% of the time, I see no value / need to have just a few lines of complicated, obscure, hard to read / maintain VBA.

**SamT** · 09-11-2017, 05:54 AM

Originally Posted by snb

100% Arrays:

Sub M_snb()
   sn = Sheets("Data").Cells(1).CurrentRegion
   sp = Sheets("Mapping").UsedRange
   
   For j = 2 To UBound(sn)
       For jj = 2 To UBound(sp)
          If sn(j, 3) = sp(jj, 1) Then
             sn(j, 17) = sp(jj, 2)
             sn(j, 18) = sp(jj, 4)
             sn(j, 19) = sp(jj, 5)
             Exit For
          End If
      Next
   
       For jj = 2 To UBound(sp)
          If sn(j, 3) = sp(jj, 16) Then
             sn(j, 20) = sp(jj, 20)
             sn(j, 21) = sp(jj, 21)
             Exit For
          End If
      Next
      
      w_00 = sn(j, 2) - Weekday(sn(j, 2))
      For jj = 22 To 28
        If jj < 27 Then sn(j, jj - 10) = Format(Choose(jj - 21, sn(j, 2), sn(j, 2), sn(j, 2), _
          w_00 + 1, w_00 + 7), Choose(jj - 21, "dddd", "'mmm-yy", "\Wk ww", _
          "\WB dd-mm-yyyy", "\WE dd-mm-yyyy"))

        sn(j, jj) = Choose(jj - 21, sn(j, 4) * sn(j, 5), sn(j, 4) * sn(j, 6), sn(j, 4) _
         * sn(j, 7), sn(j, 4) * sn(j, 8), sn(j, 9) / 600, sn(j, 10) / 60, sn(j, 11) / 60)
      Next
   Next
   
   Sheets("Data").Cells(1).CurrentRegion.Offset(19) = sn
End Sub

Sheet "mapping" columns I:N are redundant.
You don't even need screenupdating =false.

Does this do the same thing

    sn = Sheets("Data").Cells(1).CurrentRegion 
    sp = Sheets("Mapping").UsedRange 
     
    For j = 2 To UBound(sn) 
        For jj = 2 To UBound(sp) 
            If sn(j, 3) = sp(jj, 1) Then 
                sn(j, 17) = sp(jj, 2) 
                sn(j, 18) = sp(jj, 4) 
                sn(j, 19) = sp(jj, 5) 
If jj > 21 And jj < 29 Then GoTo jj2228
                Exit For 
             ElseIf sn(j, 3) = sp(jj, 16) Then 
                sn(j, 20) = sp(jj, 20) 
                sn(j, 21) = sp(jj, 21) 
If jj > 21 And jj < 29 Then GoTo jj2228
                Exit For 
            End If 

jj2228:        
If  jj > 21 And jj < 29 Then
        w_00 = sn(j, 2) - Weekday(sn(j, 2)) 

             If jj < 27 Then 
sn(j, jj - 10) = Format(Choose(jj - 21, sn(j, 2), sn(j, 2), sn(j, 2), w_00 + 1, w_00 + 7), _
Choose(jj - 21, "dddd", "'mmm-yy", "\Wk ww", "\WB dd-mm-yyyy", "\WE dd-mm-yyyy")) 
End If
            
sn(j, jj) = Choose(jj - 21, sn(j, 4) * sn(j, 5), sn(j, 4) * sn(j, 6), sn(j, 4) * sn(j, 7), sn(j, 4) _
* sn(j, 8), sn(j, 9) / 600, sn(j, 10) / 60, sn(j, 11) / 60) 
End If
        Next 
    Next 
     
    Sheets("Data").Cells(1).CurrentRegion.Offset(19) = sn 
End Sub

**Paul_Hossler** · 09-11-2017, 06:17 AM

Originally Posted by Svmaxcel

Fantastic work!!!!!
Had some questions here.
1) should we use Vlookup or Index/Match, people usually say that Index/Match is much more faster.
2) Iferror is not used in any formula, in case there Is an error(values not found), what will happen.
3) Can we create a new Sheet after the execution is done with the format attached.

1 - People say that. VLookup is simpler

2 - This fills in a "-" just in case, and the On Error Resume Next says to ignore any error, which leaves the "-"

            For i = 12 To 28
                .Cells(i).Value = "-"
            Next I
              
            On Error Resume Next

3 - Yes. See 'Roster' in the attachment

I don't like having to VLookup() the WB and WE values since they can be computed from the Date -- what are the rules to determine WB and WE?

Then you wouldn't need Mapping I:N at all

Option Explicit
'     1       2      3    4            5                  6                   7                   8                      9                           10                             11
'Skill Group Date    ID  Calls   Avg Handle Time In  Avg Talk Time In    Avg Hold Time In    Avg Wrap Time In    Not Ready Time (Per Agent)  Logged On Time (Per Agent)  Available Time (Per Agent)
' 12  13      14         15             16           17          18            19           20          21            22                  23             24               25              26                  27                      28
'Day Month   Week    Week Beginning  Week Ending Agent Name  Team Manager    Location    SkillSet    Language    Total Handle Time   Total Talk Time Total Hold Time Total Wrap Time Not Ready Time(min) Logged On Time(min) Available Time(min)

Sub AddData()
    Dim rData As Range, rRow As Range, rDates As Range, rNames As Range, rSkills As Range, rRoster As Range, rRoster1 As Range
    Dim i As Long
    Dim oWSF As WorksheetFunction
    If Not TypeOf Selection Is Range Then Exit Sub
        
    Set rData = Worksheets("Data").Range("A1").CurrentRegion
    Set rDates = Worksheets("Mapping").Range("I1").CurrentRegion
    Set rNames = Worksheets("Mapping").Range("A1").CurrentRegion
    Set rNames = rNames.Cells(1, 2).Resize(rNames.Rows.Count, rNames.Columns.Count - 1) ' emp number is in A
    Set rSkills = Worksheets("Mapping").Range("P1").CurrentRegion
    Set oWSF = Application.WorksheetFunction
    
    Application.ScreenUpdating = False
    
    For Each rRow In Intersect(Selection.EntireRow, rData).Rows
        With rRow
            If .Row = 1 Then GoTo GetNext
            If Len(.Cells(1).Value) = 0 Then GoTo GetNext
            
            For i = 12 To 28
                .Cells(i).Value = "-"
            Next i
              
            On Error Resume Next
            '  1      2    3      4           5               6
            'Date    Day Month   Week    Week Beginning  Week Ending
            .Cells(12).Value = Format(.Cells(2).Value, "DDDD")
            .Cells(13).Value = Format(.Cells(2).Value, "MMM-YY")
            .Cells(14).Value = "WK" & oWSF.WeekNum(.Cells(2).Value)
            .Cells(15).Value = oWSF.VLookup(CLng(.Cells(2).Value), rDates, 5, False)
            .Cells(16).Value = oWSF.VLookup(CLng(.Cells(2).Value), rDates, 6, False)
        
            '         1   2          3          4          5            6
            'EMP ID  ID  Name    Designation Supervisor  Location    Team Name
            .Cells(17).Value = oWSF.VLookup(.Cells(3).Value, rNames, 2, False)
            .Cells(18).Value = oWSF.VLookup(.Cells(3).Value, rNames, 4, False)
            .Cells(19).Value = oWSF.VLookup(.Cells(3).Value, rNames, 5, False)
        
            '    1            2         3        4             5
            'Skill_Name  Department  Product Skill_LOB   Skill_Language
            .Cells(20).Value = oWSF.VLookup(.Cells(1).Value, rSkills, 4, False)
            .Cells(21).Value = oWSF.VLookup(.Cells(1).Value, rSkills, 5, False)
        
            If .Cells(5).Value <> 0 Then .Cells(22).Value = .Cells(4).Value * .Cells(5).Value
            If .Cells(6).Value <> 0 Then .Cells(23).Value = .Cells(4).Value * .Cells(6).Value
            If .Cells(7).Value <> 0 Then .Cells(24).Value = .Cells(4).Value * .Cells(7).Value
            If .Cells(8).Value <> 0 Then .Cells(25).Value = .Cells(4).Value * .Cells(8).Value
            .Cells(26).Value = .Cells(9).Value / 600
            .Cells(27).Value = .Cells(10).Value / 60
            .Cells(28).Value = .Cells(11).Value / 60
            On Error GoTo 0
        
        End With
GetNext:
    Next
    'make roster worksheet
    On Error Resume Next
    Application.DisplayAlerts = False
    Worksheets("Roster").Delete
    Application.DisplayAlerts = True
    On Error GoTo 0
    Worksheets.Add.Name = "Roster"
    With rData
        .Columns(2).Copy Worksheets("Roster").Range("A1")
        .Columns(3).Copy Worksheets("Roster").Range("B1")
        .Columns(17).Copy Worksheets("Roster").Range("C1")
        .Columns(18).Copy Worksheets("Roster").Range("D1")
    End With
    
    Set rRoster = Worksheets("Roster").Range("A1").CurrentRegion
    With rRoster
        .EntireColumn.AutoFit
        Set rRoster1 = .Cells(2, 1).Resize(.Rows.Count - 1, .Columns.Count)
    End With
    
    With Worksheets("Roster").Sort
        .SortFields.Clear
        .SortFields.Add Key:=rRoster1.Columns(1), SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
        .SortFields.Add Key:=rRoster1.Columns(2), SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
        .SetRange rRoster
        .Header = xlYes
        .MatchCase = False
        .Orientation = xlTopToBottom
        .SortMethod = xlPinYin
        .Apply
    End With
    Application.ScreenUpdating = True
End Sub

**snb** · 09-11-2017, 06:31 AM

@Sam T

No.

the first 2 loops filter in array sp (the counter jj represents the rows in array sp)

the last loop is independent of array sp. The counter jj represents the 'column' in array sn.

**Svmaxcel** · 09-11-2017, 07:03 AM

No words to describe your hard and superb work.
You guys are God of Excel and VBA

**snb** · 09-11-2017, 07:40 AM

Sub M_snb() 
  sn = Sheets("Data").Cells(1).CurrentRegion 
  sp = Sheets("Mapping").UsedRange 
  sf= split("dddd_'mmm-yy_\Wk ww_\WB dd-mm-yyyy_\WE dd-mm-yyyy","_")
 
  For j = 2 To UBound(sn) 
    For jj = 2 To UBound(sp) 
      If sn(j, 3) = sp(jj, 1) Then Exit for
    next
    sn(j, 17) = sp(jj, 2) 
    sn(j, 18) = sp(jj, 4) 
    sn(j, 19) = sp(jj, 5) 
         
    For jj = 2 To UBound(sp) 
      If sn(j, 1) = sp(jj, 16) Then exit for
    next
    sn(j, 20) = sp(jj, 20) 
    sn(j, 21) = sp(jj, 21) 
         
    w_00 = sn(j, 2) - Weekday(sn(j, 2)) 
    For jj = 22 To 28 
      If jj < 27 Then sn(j, jj - 10) = Format(Choose(jj - 21, sn(j, 2), sn(j, 2), sn(j, 2), w_00 + 1, w_00 + 7), sf(jj - 21)) 
      sn(j, jj) = Choose(jj - 21, sn(j, 5) * sn(j, 4), sn(j, 6) * sn(j, 4), sn(j, 7) * sn(j, 4), sn(j, 8) * sn(j, 4), sn(j, 9) / 600, sn(j, 10) / 60, sn(j, 11) / 60) 
    Next 
  Next 
     
  Sheets("Data").Cells(1).CurrentRegion.Offset(19) = sn 
End Sub

**offthelip** · 09-11-2017, 08:46 AM

Paul;

I deliberately avoided a complicated, sophisticated VBA array approach since (while some members here could maintain it) I wanted the OP to have something simple that worked, even if not the highest performance

Personal Opinion: 99.9999% of the time, I see no value / need to have just a few lines of complicated, obscure, hard to read / maintain VBA.

I agree with you totally except in this case where the title of the thread is "Excel Slow performance" in which case I believe the OP has expressed interest in knowing what is the fast way of doing things.

**snb** · 09-11-2017, 09:01 AM

When you apply F8 and the local window, I think the array approach is much easier to follow than the Excelformulae-in-VBA approach.
Maybe it takes some time the first time, but when grasping the code sets in, the learning process is much faster ( own experience).

**Paul_Hossler** · 09-11-2017, 09:11 AM

Originally Posted by offthelip

Paul;

I agree with you totally except in this case where the title of the thread is "Excel Slow performance" in which case I believe the OP has expressed interest in knowing what is the fast way of doing things.

I agree with you agreeing with me, and if each day or every time 300K+ lines might have to be processes, I'd re-think.

Since IN THIS CASE it appears that a relatively small amount data was appended incrementally daily or several times a day, IMVVVHO a simple, user-maintainable approach seemed best

It's a tradeoff

**Svmaxcel** · 09-11-2017, 09:19 AM

Thanks buddy.
You are write Vlookup is Simpler, but does it actually slows down things when compared to Index Match, if Vlookup is better option I don't mind using it.

There are no specific rules in WB and WB,
WB dates starts From Sunday
WE dates starts from Saturday

Attaching file from reference

**Paul_Hossler** · 09-11-2017, 09:21 AM

Originally Posted by snb

When you apply F8 and the local window, I think the array approach is much easier to follow than the Excel formulae-in-VBA approach.
Maybe it takes some time the first time, but when grasping the code sets in, the learning process is much faster (own experience).

For a very experienced coder (such as yourself) who is familiar with your coding style and arrays processing, and who codes for a living (or hobby??) I'd probably agree

However, for us regular people who might not look at the macro for a year (and only when there's a problem, or something had to be added) , I believe that it's better to err on the side of wordy-ness and simplicity as much as possible, even if performance takes a hit.

To mitigate the performance impact, I made the macro 'smart' (OK, only my opinion) enough to just use the selected 1,000 rows, instead of recalculating the other 299,000 rows

Running the non-array macro on the 1000 rows just added, would most likely have an imperceptible delay

Just my $0.02

**Paul_Hossler** · 09-11-2017, 09:53 AM

Originally Posted by Svmaxcel

Thanks buddy.
You are write Vlookup is Simpler, but does it actually slows down things when compared to Index Match, if Vlookup is better option I don't mind using it.

There are no specific rules in WB and WB,
WB dates starts From Sunday
WE dates starts from Saturday

Well, I don't know if VLookup is slower than Match/Index, but I added the VBA equivalent (using arrays for the Mapping tables) A collection or dictionary seems like over-kill

I compute the WB/WE dates -- note that you have different formats for some

If you still need more performance, then I think you should go with one of the totally array approaches that others have suggested

Option Explicit
'     1       2      3    4            5                  6                   7                   8                      9                           10                             11
'Skill Group Date    ID  Calls   Avg Handle Time In  Avg Talk Time In    Avg Hold Time In    Avg Wrap Time In    Not Ready Time (Per Agent)  Logged On Time (Per Agent)  Available Time (Per Agent)
' 12  13      14         15             16           17          18            19           20          21            22                  23             24               25              26                  27                      28
'Day Month   Week    Week Beginning  Week Ending Agent Name  Team Manager    Location    SkillSet    Language    Total Handle Time   Total Talk Time Total Hold Time Total Wrap Time Not Ready Time(min) Logged On Time(min) Available Time(min)

Sub AddData()
    Dim rData As Range, rRow As Range, rNames As Range, rSkills As Range, rRoster As Range, rRoster1 As Range
    Dim i As Long
    Dim oWSF As WorksheetFunction
    Dim vIDs As Variant, vNames As Variant
    Dim vSkill As Variant, vSkills As Variant
    
    If Not TypeOf Selection Is Range Then Exit Sub
        
    Set oWSF = Application.WorksheetFunction
    
    Set rData = Worksheets("Data").Range("A1").CurrentRegion
    
    Set rNames = Worksheets("Mapping").Range("A1").CurrentRegion
    Set rNames = rNames.Cells(1, 2).Resize(rNames.Rows.Count, rNames.Columns.Count - 1) ' emp number is in A
    vIDs = oWSF.Transpose(rNames.Columns(1).Value)
    vNames = rNames.Value
    
    Set rSkills = Worksheets("Mapping").Range("I1").CurrentRegion
    vSkill = oWSF.Transpose(rSkills.Columns(1).Value)
    vSkills = rSkills.Value
    Application.ScreenUpdating = False
    
    For Each rRow In Intersect(Selection.EntireRow, rData).Rows
        With rRow
            If .Row = 1 Then GoTo GetNext
            If Len(.Cells(1).Value) = 0 Then GoTo GetNext
            
            For i = 12 To 28
                .Cells(i).Value = "-"
            Next I
              
            On Error Resume Next
            '  1      2    3      4           5               6
            'Date    Day Month   Week    Week Beginning  Week Ending
            .Cells(12).Value = Format(.Cells(2).Value, "DDDD")
            .Cells(13).Value = Format(.Cells(2).Value, "MMM-YY")
            .Cells(14).Value = "WK" & oWSF.WeekNum(.Cells(2).Value)
            .Cells(15).Value = "WB " & Format(.Cells(2).Value - oWSF.Weekday(.Cells(2).Value) + 1, "dd-mmm")
            .Cells(16).Value = "WE " & Format(.Cells(2).Value - oWSF.Weekday(.Cells(2).Value) + 7, "dd-mmm")
        
            '         1   2          3          4          5            6
            'EMP ID  ID  Name    Designation Supervisor  Location    Team Name
            i = oWSF.Match(.Cells(3).Value, vIDs, 0)
            .Cells(17).Value = vNames(i, 2)
            .Cells(18).Value = vNames(i, 4)
            .Cells(19).Value = vNames(i, 5)
        
            '    1            2         3        4             5
            'Skill_Name  Department  Product Skill_LOB   Skill_Language
            i = oWSF.Match(.Cells(1).Value, vSkill, 0)
            .Cells(20).Value = vSkills(i, 4)
            .Cells(21).Value = vSkills(i, 5)
        
            If .Cells(5).Value <> 0 Then .Cells(22).Value = .Cells(4).Value * .Cells(5).Value
            If .Cells(6).Value <> 0 Then .Cells(23).Value = .Cells(4).Value * .Cells(6).Value
            If .Cells(7).Value <> 0 Then .Cells(24).Value = .Cells(4).Value * .Cells(7).Value
            If .Cells(8).Value <> 0 Then .Cells(25).Value = .Cells(4).Value * .Cells(8).Value
            .Cells(26).Value = .Cells(9).Value / 600
            .Cells(27).Value = .Cells(10).Value / 60
            .Cells(28).Value = .Cells(11).Value / 60
            On Error GoTo 0
        
        End With
GetNext:
    Next
    'make roster worksheet
    On Error Resume Next
    Application.DisplayAlerts = False
    Worksheets("Roster").Delete
    Application.DisplayAlerts = True
    On Error GoTo 0
    Worksheets.Add.Name = "Roster"
    With rData
        .Columns(2).Copy Worksheets("Roster").Range("A1")
        .Columns(3).Copy Worksheets("Roster").Range("B1")
        .Columns(17).Copy Worksheets("Roster").Range("C1")
        .Columns(18).Copy Worksheets("Roster").Range("D1")
    End With
    
    Set rRoster = Worksheets("Roster").Range("A1").CurrentRegion
    With rRoster
        .EntireColumn.AutoFit
        Set rRoster1 = .Cells(2, 1).Resize(.Rows.Count - 1, .Columns.Count)
    End With
    
    With Worksheets("Roster").Sort
        .SortFields.Clear
        .SortFields.Add Key:=rRoster1.Columns(1), SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
        .SortFields.Add Key:=rRoster1.Columns(2), SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
        .SetRange rRoster
        .Header = xlYes
        .MatchCase = False
        .Orientation = xlTopToBottom
        .SortMethod = xlPinYin
        .Apply
    End With
    Application.ScreenUpdating = True
End Sub

**SamT** · 09-11-2017, 11:53 AM

@ sb,

Oh, I see. THanks for checking me.

**snb** · 09-11-2017, 01:26 PM

Using 2 two-dimensional arrays and 1 1-dimensional array I can't call 'complicated'.
Nor maintaining 22 lines of code of which most of the lines have the pattern x=y.
Even after 7 seven years.

**SamT** · 09-11-2017, 02:28 PM

@ Paul

However, for us regular people who might not look at the macro for a year (and only when there's a problem, or something had to be added) , I believe that it's better to err on the side of wordy-ness and simplicity as much as possible,

Just rename his variables to meaningful names and it is much easier to read and parse.

    arrData = Sheets("Data").Cells(1).CurrentRegion 
    arrMapping = Sheets("Mapping").UsedRange 
    arrFormats= split("dddd_'mmm-yy_\Wk ww_\WB dd-mm-yyyy_\WE dd-mm-yyyy","_") 
     
    For DataRow = 2 To UBound(arrData) 
        For MappingRow = 2 To UBound(arrMapping) 
            If arrData(DataRow, 3) = arrMapping(MappingRow, 1) Then Exit For 
        Next 
        arrData(DataRow, 17) = arrMapping(MappingRow, 2) 
        arrData(DataRow, 18) = arrMapping(MappingRow, 4) 
        arrData(DataRow, 19) = arrMapping(MappingRow, 5) 
         
        For MappingRow = 2 To UBound(arrMapping) 
            If arrData(DataRow, 1) = arrMapping(MappingRow, 16) Then exit For 
        Next 
        arrData(DataRow, 20) = arrMapping(MappingRow, 20) 
        arrData(DataRow, 21) = arrMapping(MappingRow, 21) 
         
        A_Day = arrData(DataRow, 2) - Weekday(arrData(DataRow, 2)) 
        For MappingRow = 22 To 28 'Possibly, no, probably, misnamed
            If MappingRow < 27 Then arrData(DataRow, MappingRow - 10) = Format(Choose(MappingRow - 21, arrData(DataRow, 2), arrData(DataRow, 2), arrData(DataRow, 2), A_Day + 1, A_Day + 7), arrFormats(MappingRow - 21)) 
            arrData(DataRow, MappingRow) = Choose(MappingRow - 21, arrData(DataRow, 5) * arrData(DataRow, 4), arrData(DataRow, 6) * arrData(DataRow, 4), arrData(DataRow, 7) * arrData(DataRow, 4), arrData(DataRow, 8) * arrData(DataRow, 4), arrData(DataRow, 9) / 600, arrData(DataRow, 10) / 60, arrData(DataRow, 11) / 60) 
        Next 
    Next 
     
    Sheets("Data").Cells(1).CurrentRegion.Offset(19) = arrData

If you want to go farther, you can take most of the Magic Numbers and replace them with Constants/Enums named after the respective Column heads

**offthelip** · 09-11-2017, 02:58 PM

When you apply F8 and the local window, I think the array approach is much easier to follow than the Excelformulae-in-VBA approach.
Maybe it takes some time the first time, but when grasping the code sets in, the learning process is much faster ( own experience).

I think snb has a very good point, when you look at the code it doesn't look any more complicated. If one has any experience using other programming languages then using arrays looks perfectly normal, it is the EXCEL bit, of ranges and cells that looks different.
Also because using arrays is so much faster than using ranges, I think it is worth learning how to do it. This is partly because doing things differently only when you need extra speed is not a very efficient way of working, especially when you only discover you need the extra speed having programmed it the slow way. Thus requiring reprogramming.
I always use variant arrays whenever I can. I never know when I might reuse a bit of code which was orignally coded as a once through routine but now becomes a multi loop routine.
Note; I do accept that my opinion is totally biased since I have been using computers for 49 years, in more languages than I could possibly count.

**Paul_Hossler** · 09-11-2017, 06:30 PM

Note; I do accept that my opinion is totally biased since I have been using computers for 49 years, in more languages than I could possibly count.

@offthelip --

And my opinion isn't????

I mended punch paper tapes with scotch tape to feed them into the reader

Then they came out with the new fangled 'key-to-disc' technology that eliminated having to punch cards and worry about dropping the box in the hallway

@SamT --

po-ta-to, po-tah-o

Yes, you and snb are perfectly correct -- you and snb and I could figure it out. Maybe the OP could also

If I were doing it for myself, I would have done it differently: Consts or eNums, an array (possibly) for the row, intelligent determination of what new data requires filling in, etc.

The examples and suggestions all seem to recompute data that has be computed.

My approach allows the selection of data to be computed. Yes, I'm sure that logic could be added to build the data array with only data that needs to be computed and logic to build the requested Roster sheet

Bottom Line:

I agree that we disagree

**snb** · 09-12-2017, 01:02 AM

We can't disagree on:

using 'select' slows down the code
using 'activate' slows down the code
worksheet interaction slows down the code

In the perspective of 'speed' (see the thread title) these ar the first 3 most influential recommendations: avoid 'select', avoid 'activate', avoid worksheet/document interaction.

After that improvements can be obtained by restructuring data, reducing loops,etc. but these are minor compared to the first 3 recommendations.

If you add the perspective 'TS knowledge' you introduce a perspective that is highly based on assumptions.
I can't tell by alias the competence level ot the TS.
From the start I therefore assume 'complete competence'.
That's why I give an answer without taking the 'TS competence' into account.
Only after the reaction of the TS on the provided suggestion I can make some assessment of his/her competence level.
If I get the impression the TS is intrigued by the suggestion and is willing to learn more I provide more explanation.

I also keep in mind that our suggestions are not only meant to serve the TS but also the 'thousands' of visitors [irony]that are looking for 'excellent examples' of VBA solutions in the decades to come[/irony].
The competence level of these visitors I am not aware of; so I prefer to provide them a suggestion that meets VBA criteria first.

BTW. rereading my code I still found 1 error (since it is so simple to read it is very simple to debug):

Sub M_snb() 
    sn = Sheets("Data").Cells(1).CurrentRegion 
    sp = Sheets("Mapping").UsedRange 
    sf= split("dddd_'mmm-yy_\Wk ww_\WB dd-mm-yyyy_\WE dd-mm-yyyy","_") 
     
    For j = 2 To UBound(sn) 
        For jj = 2 To UBound(sp) 
            If sn(j, 3) = sp(jj, 1) Then Exit For 
        Next 
        sn(j, 17) = sp(jj, 2) 
        sn(j, 18) = sp(jj, 4) 
        sn(j, 19) = sp(jj, 5) 
         
        For jj = 2 To UBound(sp) 
            If sn(j, 1) = sp(jj, 16) Then exit For 
        Next 
        sn(j, 20) = sp(jj, 20) 
        sn(j, 21) = sp(jj, 21) 
         
        w_00 = sn(j, 2) - Weekday(sn(j, 2)) 
        For jj = 22 To 28 
            If jj < 27 Then sn(j, jj - 10) = Format(Choose(jj - 21, sn(j, 2), sn(j, 2), sn(j, 2), w_00 + 1, w_00 + 7), sf(jj - 22)) 
            sn(j, jj) = Choose(jj - 21, sn(j, 5) * sn(j, 4), sn(j, 6) * sn(j, 4), sn(j, 7) * sn(j, 4), sn(j, 8) * sn(j, 4), sn(j, 9) / 600, sn(j, 10) / 60, sn(j, 11) / 60) 
        Next 
    Next 
     
    Sheets("Data").Cells(1).CurrentRegion.Offset(19) = sn 
End Sub

**SamT** · 09-12-2017, 06:44 AM

?

        For jj = 2 To UBound(sp) 
            If sn(j, 3) = sp(jj, 1) Then Exit For 
        Next

You use that construct twice:

        For jj = 2 To UBound(sp) 
            If sn(j, 1) = sp(jj, 16) Then exit For 
        Next

since it is so simple to read

I had 4 questions about one line of your code. After reading the help files and rereading that line, (and some of the preceding ones,) many, many times, I answered all of them

Don't get me wrong, I've "stolen" more of your code than any one else's, but the first thing I do is Ctrl+H every variable.

Thread: Excel Slow performance

Thread Tools

Display

Posting Permissions