I am trying to populate a table like this in Excel. I came up with a bunchof loops to do this. I feel like this is a brute-force approach and may not work well when populating bigger tables.
Any other ways to populate values in Excel?
Sub Fill_SLA_priority() Dim SLAbyPrior As Worksheet Dim wsAssignedTix As Worksheet Dim ctP1 As Integer Dim ctP1RespSLA As Integer Dim ctP1PlanSLA As Integer Dim ctP1ResSLA As Integer Dim ctP1All3SLA As Integer Dim d As Range Set SLAbyPrior = Worksheets("SLA by Priority") Set wsAssignedTix = Worksheets("AssignedTickets") For Each d In wsAssignedTix.Range(wsAssignedTix.Range("H1").End(xlDown), wsAssignedTix.Range("H" & Rows.count).End(xlUp)) '' Loop through entire list to count number of P1 - P4, Request. '' P1 '' Count number of Responded, Planned, Resolved, Overall SLA If d.Value = "Priority 1 Incident - Critical" _ And (d.Offset(0, -1) = "Closed" Or d.Offset(0, -1) = "Resolved") Then ctP1 = ctP1 + 1 If d.Offset(0, 2) = 1 Then ctP1RespSLA = ctP1RespSLA + 1 If d.Offset(0, 4) = 1 Then ctP1PlanSLA = ctP1PlanSLA + 1 If d.Offset(0, 6) = 1 Then ctP1ResSLA = ctP1ResSLA + 1 If d.Offset(0, 2) = 1 And d.Offset(0, 4) = 1 And d.Offset(0, 6) = 1 Then ctP1All3SLA = ctP1All3SLA + 1 End If End If Next d '' Populating the cells in SLA_by_Priority With SLAbyPrior '' P1 - Resp SLA % .Range("B5") = ctP1RespSLA / ctP1 '' P1 - Plan SLA % .Range("C5") = ctP1PlanSLA / ctP1 '' P1 - Resl SLA % .Range("D5") = ctP1ResSLA / ctP1 '' P1 - All 3 SLA % .Range("E5") = ctP1All3SLA / ctP1 End WithEnd Sub
1 Answer1
First things first -variable naming - help me. I look at your variables and have no idea what they are, what they are meant to do or how they should be used.
Dim SLAbyPrior As WorksheetDim wsAssignedTix As WorksheetDim ctP1 As IntegerDim ctP1RespSLA As IntegerDim ctP1PlanSLA As IntegerDim ctP1ResSLA As IntegerDim ctP1All3SLA As IntegerDim d As Range- Worksheets have a
CodeNameproperty - View Properties window (F4) and the(Name)field (the one at the top) can be used as the worksheet name. This way you can avoidset wsAssignedTix = Sheets("AssignedTickets")and instead just useAssignedTickets. - Integers -integers are obsolete. According tomsdn VBAsilently converts all integers to
long.
Let's try to decipher what your variables are and give them meaningful names
d = priorityValuectP1 = countOfCritical?ctP1RespSLA - countOfCriticalResponseServiceLevelAgreement?Actually, I can't decipher them. Sorry.
For Each d In wsAssignedTix.Range(wsAssignedTix.Range("H1").End(xlDown), wsAssignedTix.Range("H" & Rows.Count).End(xlUp))This could be done differently:
Dim lastRow As LonglastRow = wsAssignedTix.Cells(Rows.Count, 8).End(xlUp).RowDim i As LongFor i = 1 To lastRowNow change yourif
Const CRITICAL_PRIORITY As String = "Priority 1 Incident - Critical"Const CLOSED_OR_RESOLVED As String = "Closed Resolved"If Cells(i, 8) = CRITICAL_PRIORITY And InStr(1, CLOSED_OR_RESOLVED, Cells(i, 7).Value) > 0 ThenNow do whatever all that adding is, but you can avoid theoffset by just using the column number with the current row.
I'd also avoid doing the calculation in theWith block by using a variable, personally.
Without really understanding what you're calculating, and assuming your values are either1 or0 you can probably just sum as you go rather than testing each offset cell for1.
And, most likely, you'd benefit from loading the tickets sheet into an array and calculating on the array rather than the sheet.
Actually, what you could probably do is sort the tickets by critical and then just sum the columns for each status based on the count of critical tickets. That would be simpler.
You mustlog in to answer this question.
Explore related questions
See similar questions with these tags.