HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Iterate through slicer settings and print to PDF

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
settingssliceriterateprintthroughandpdf

Problem

I have a spreadsheet with seven tables (tbl_1, tbl_2, ..., tbl_7) each controlled by its own slicer. Each slicer has six buttons (10, 20, 30, 40, 50, 60) referring to Team Codes. I use the code below to select one team on every slicer, then create a PDF for each team / slicer setting.

As of now, the code takes anywhere from 5-7min to run.

Sub SlicerTeam()
Dim wb As Workbook
Dim sc As SlicerCache
Dim si As SlicerItem

On Error GoTo errHandler
Application.ScreenUpdating = False
Application.EnableEvents = False

Set wb = ThisWorkbook

For x = 1 To 6
    For i = 1 To 7
    Set sc = wb.SlicerCaches("tbl_" & i)
        sc.ClearAllFilters
        For Each si In sc.VisibleSlicerItems
            Set si = sc.SlicerItems(si.Name)
                If Not si Is Nothing Then
                    If si.Name = x * 10 Then
                        si.Selected = True
                    Else
                        si.Selected = False
                    End If
                Else
                    si.Selected = False
                End If
        Next si

    Next i
Call PDFCreate
Next x

exitHandler:
Application.ScreenUpdating = True
Application.EnableEvents = True
Exit Sub

errHandler:
MsgBox ("Error in updating slicer filters.")
Resume exitHandler

End Sub

Sub PDFCreate()
Dim Fname As String
Dim path As String

path = "S:\MyFilePath\"
Fname = path & Sheets("Detail").Range("T1").Value & " - " & [TEXT('Population Detail'!B1,"mmm, yyyy")] & ".pdf"

    Sheets("Detail").ExportAsFixedFormat Type:=xlTypePDF, _
        FileName:=Fname, Quality:=xlQualityStandard, IncludeDocProperties:=True, _
        IgnorePrintAreas:=False, OpenAfterPublish:=False

End Sub

Solution

Variables

Let's discuss variables a bit.

-
Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.

-
When you don't define your variable, VBA will declare it as a Variant type that can hold any type of data. While this may be more flexible, it adds processing time to your macro as VBA decides or tests for the type. Additionally, since a Variant can be any type of data, you may miss out on valuable troubleshooting information on Type Mismatch

-
Variable names - give your variables meaningful names.

-
Personal preference - I avoid variables like i and j because they aren't descriptive. But, there's technically nothing wrong with using them as it's standard practice to iterate with i.

-
Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.

That being said -

You don't declare x or i. But those aren't vary descriptive are they? Try:

Dim slicerButtonNumber As Long
Dim tableNumber As Long


You don't have great names for the variables you do declare -

wb - targetWorkbook
sc - targetSlicerCache
si - targetSlicerItem


Extending the names using descriptive words costs you nothing and saves you headaches!

Also, here:

Set targetSlicerCache = targetBook.SlicerCaches("tbl_" & tableNumber)


You're using a constant - "tbl_", so

Const TABLE_PREFIX as String = "tbl_"
Set targetSlicerCache = targetBook.SlicerCaches(TABLE_PREFIX & tableNumber)


It may not seem like much, but by declaring that at top, if it ever changes, you only need to change it in one place.

Same goes for PDFCreate:

Const SHEET_NAME As String = "Detail"
Const POPULATION_SHEET_NAME As String = "Population Detail"
Const SEPARATOR As String = " - "
Dim detailSheet As Worksheet
Set detailSheet = ThisWorkbook.Sheets(SHEET_NAME)
Dim populationSheet As Worksheet
Set populationSheet = ThisWorkbook.Sheets(POPULATION_SHEET_NAME)
Dim file_Name As String
Dim file_Path As String
Dim aPathValue As String

file_Path = "S:\MyFilefilePath\"
aPathValue = detailSheet.Range("T1").Value & SEPARATOR
file_Path = file_Path & aPathValue
file_Name = filePath & [TEXT(populationsheet.range("B1"),"mmm, yyyy")] & ".pdf"

detailSheet.ExportAsFixedFormat Type:=xlTypePDF, _
    filename:=file_Name, Quality:=xlQualityStandard, IncludeDocProperties:=True, _
    IgnorePrintAreas:=False, OpenAfterPublish:=False


You don't need to Call subs, it's obsolete. Instead just use Sub argument, argument

Speed

Your set here is redundant

For Each targetSlicerItem In sc.VisibleSlicerItems
            Set targetSlicerItem = targetSlicerCache.SlicerItems(targetSlicerItem.Name)


The For sets the items.

This is also unnecessary:

If Not targetSlicerItem Is Nothing Then


It won't be Nothing since you're only addressing the items that exist in the cache.

You can condense down

For slicerButtonNumber = 1 To 6
    For tableNumber = 1 To 7
    Set targetSlicerCache = targetBook.SlicerCaches(TABLE_PREFIX & tableNumber)
        targetSlicerCache.ClearAllFilters
        For Each targetSlicerItem In targetSlicerCache.VisibleSlicerItems
            If targetSlicerItem.Name = slicerButtonNumber * 10 Then
                targetSlicerItem.Selected = True
            Else
                targetSlicerItem.Selected = False
        End If
        Next targetSlicerItem
    Next i


You can however, remove one of your loops, the slicerButtonNumber loop

Const BUTTON_NUMBERS As String = "10,20,30,40,50,60"
Const DELIMITER As String = ","
Dim slicerButtonNumbers As Variant
slicerButtonNumbers = Split(BUTTON_NUMBERS, DELIMITER)
    

 For tableNumber = 1 To 7
    Set targetSlicerCache = targetBook.SlicerCaches(TABLE_PREFIX & tableNumber)
        targetSlicerCache.ClearAllFilters
        For Each targetSlicerItem In targetSlicerCache.VisibleSlicerItems
            If UBound(Filter(slicerButtonNumbers, targetSlicerItem.Name)) > -1 Then
               targetSlicerItem.Selected = True
            Else
                targetSlicerItem.Selected = False
            End If
        Next targetSlicerItem
    Next tableNumber

Code Snippets

Dim slicerButtonNumber As Long
Dim tableNumber As Long
wb - targetWorkbook
sc - targetSlicerCache
si - targetSlicerItem
Set targetSlicerCache = targetBook.SlicerCaches("tbl_" & tableNumber)
Const TABLE_PREFIX as String = "tbl_"
Set targetSlicerCache = targetBook.SlicerCaches(TABLE_PREFIX & tableNumber)
Const SHEET_NAME As String = "Detail"
Const POPULATION_SHEET_NAME As String = "Population Detail"
Const SEPARATOR As String = " - "
Dim detailSheet As Worksheet
Set detailSheet = ThisWorkbook.Sheets(SHEET_NAME)
Dim populationSheet As Worksheet
Set populationSheet = ThisWorkbook.Sheets(POPULATION_SHEET_NAME)
Dim file_Name As String
Dim file_Path As String
Dim aPathValue As String

file_Path = "S:\MyFilefilePath\"
aPathValue = detailSheet.Range("T1").Value & SEPARATOR
file_Path = file_Path & aPathValue
file_Name = filePath & [TEXT(populationsheet.range("B1"),"mmm, yyyy")] & ".pdf"

detailSheet.ExportAsFixedFormat Type:=xlTypePDF, _
    filename:=file_Name, Quality:=xlQualityStandard, IncludeDocProperties:=True, _
    IgnorePrintAreas:=False, OpenAfterPublish:=False

Context

StackExchange Code Review Q#155550, answer score: 2

Revisions (0)

No revisions yet.