PDA

View Full Version : is there a better way to write this code?



ironj32
09-09-2008, 06:23 AM
hello everyone. i have the following code...it gets the job done, but seems to be a tad slow in its' execution. this code will be repeated mulitple times in my work book (using different ranges/cells/fill colors/etc.), so i would like to ensure that i am using the most proper technique/code. any help or recomendations would be greatly appreciated. thanks!



Option Explicit
Sub calcSLA_IRques_met()
IRques_metSLA_camp1
IRques_metSLA_camp2
IRques_metSLA_camp3
IRques_metSLA_camp4
IRques_metSLA_camp5
End Sub

Function IRques_metSLA_camp1()
Dim Col As Long
Dim x As Integer, n As Integer
Dim arr(56)
Dim cel As Range
Dim ws As Worksheet
Set ws = Worksheets("Summary")
For Each cel In Sheets("Detail").Range("ir_finish1")
Col = cel.Interior.ColorIndex
arr(Col) = arr(Col) + 1
Next
x = arr(3) + arr(4)
n = Application.CountIf(Range("IRcomp1_dayslate"), ">0")
ws.Range("irComp_metSLA1").Value = (x - n) / x
End Function

Function IRques_metSLA_camp2()
Dim Col As Long
Dim x As Integer, n As Integer
Dim arr(56)
Dim cel As Range
Dim ws As Worksheet
Set ws = Worksheets("Summary")
For Each cel In Sheets("Detail").Range("ir_finish2")
Col = cel.Interior.ColorIndex
arr(Col) = arr(Col) + 1
Next
x = arr(3) + arr(4)
n = Application.CountIf(Range("IRcomp2_dayslate"), ">0")
ws.Range("irComp_metSLA2").Value = (x - n) / x
End Function

Function IRques_metSLA_camp3()
Dim Col As Long
Dim x As Integer, n As Integer
Dim arr(56)
Dim cel As Range
Dim ws As Worksheet
Set ws = Worksheets("Summary")
For Each cel In Sheets("Detail").Range("ir_finish3")
Col = cel.Interior.ColorIndex
arr(Col) = arr(Col) + 1
Next
x = arr(3) + arr(4)
n = Application.CountIf(Range("IRcomp3_dayslate"), ">0")
ws.Range("irComp_metSLA3").Value = (x - n) / x
End Function

Function IRques_metSLA_camp4()
Dim Col As Long
Dim x As Integer, n As Integer
Dim arr(56)
Dim cel As Range
Dim ws As Worksheet
Set ws = Worksheets("Summary")
For Each cel In Sheets("Detail").Range("ir_finish4")
Col = cel.Interior.ColorIndex
arr(Col) = arr(Col) + 1
Next
x = arr(3) + arr(4)
n = Application.CountIf(Range("IRcomp4_dayslate"), ">0")
ws.Range("irComp_metSLA4").Value = (x - n) / x
End Function

Function IRques_metSLA_camp5()
Dim Col As Long
Dim x As Integer, n As Integer
Dim arr(56)
Dim cel As Range
Dim ws As Worksheet
Set ws = Worksheets("Summary")
For Each cel In Sheets("Detail").Range("ir_finish5")
Col = cel.Interior.ColorIndex
arr(Col) = arr(Col) + 1
Next
x = arr(3) + arr(4)
n = Application.CountIf(Range("IRcomp5_dayslate"), ">0")
ws.Range("irComp_metSLA5").Value = (x - n) / x
End Function

Bob Phillips
09-09-2008, 06:30 AM
Try turning off screenupdating and setting calculation mode to manual, and reset at the end.

ironj32
09-09-2008, 07:04 AM
thanks xld. i put in the following, but do not notice any difference. i'm okay with it...just wanted to know if i had any glaring mistakes in my vba.


Sub calcSLA_IRvalid_met()
Application.Calculation = xlCalculationManual
IRvalid_metSLA_camp1
IRvalid_metSLA_camp2
IRvalid_metSLA_camp3
IRvalid_metSLA_camp4
IRvalid_metSLA_camp5
Application.Calculation = xlCalculationAutomatic
End Sub

Bob Phillips
09-09-2008, 07:14 AM
I can't see anything obvious. They are all different ranges, and there are a few tweaks that could be made, but not significant.

Just one thing, why are you loading each cell colour into an array when you only use elements 3 & 4, couldn't you go direct to the cells and avoid the loop?