3
\$\begingroup\$

I am trying to populate a table like this in Excel. I came up with a bunch of 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 With

End Sub
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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 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
  1. Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid set wsAssignedTix = Sheets("AssignedTickets") and instead just use AssignedTickets.
  2. Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.

Let's try to decipher what your variables are and give them meaningful names

d = priorityValue
ctP1 = 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 Long
lastRow = wsAssignedTix.Cells(Rows.Count, 8).End(xlUp).Row
Dim i As Long
For i = 1 To lastRow

Now change your if

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 Then

Now do whatever all that adding is, but you can avoid the offset by just using the column number with the current row.

I'd also avoid doing the calculation in the With block by using a variable, personally.


Without really understanding what you're calculating, and assuming your values are either 1 or 0 you can probably just sum as you go rather than testing each offset cell for 1.


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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.