Creating a PowerPoint in vba by calling "slide" subroutines

Question

I am creating a powerpoint in VBA, but there are a lot of slides in it.

I was thinking I could create a macro that calls a lot of subroutines. Within those subroutines, the actual "slide code" would run. However, I have only created powerpoints where all of the code was in the same subroutine.

I took a shot at it and the below code creates what I want.

My question is, Public and Private variables and the use of "Dim". Would it be bad code if I put all of this in one module and use a bunch of subroutines? Or should I separate over modules? I would prefer to do that so I do not have to define each variable such as ppSlide and ppPres, but open to any suggestions. I am mainly looking for ideas on how to set this up.

Public ppApp As PowerPoint.Application
Public ppPres As PowerPoint.Presentation
Public ppSlide As PowerPoint.Slide

sub CreatePres()
Dim ppTextbox As PowerPoint.Shape
Dim slidesCount As Long
Set ppApp = New PowerPoint.Application
ppApp.Visible = True
ppApp.Activate

Set ppPres = ppApp.Presentations.Add
slidesCount = ppPres.Slides.Count
Set ppSlide = ppPres.Slides.Add(slidesCount + 1, ppLayoutTitle)
ppSlide.Shapes(1).TextFrame.TextRange.Text = "Hello world"
ppSlide.Shapes(2).TextFrame.TextRange.Text = Date
slidesCount = slidesCount + 1

Call slide2(slidesCount)

End Sub

Sub slide2(i As Integer)
'Insert CPR speeds Chart and Table --- slide 2
Set ppSlide = ppPres.Slides.Add(i+ 1, ppLayoutBlank)
ppSlide.Select
Worksheets("Sheet1").Activate
ActiveSheet.Range(Cells(1, 4), Cells(2, 9)).Copy
ppSlide.Shapes.Paste.Select
ppSlide.Shapes(1).Width = _
    (ppPres.PageSetup.SlideWidth / 2)
ppSlide.Shapes(1).Left = _
    (ppPres.PageSetup.SlideWidth / 4)
ppSlide.Shapes(1).Top = _
    4 * (ppPres.PageSetup.SlideHeight / 5)

ActiveSheet.ChartObjects("Chart1").Activate
ActiveChart.ChartArea.Copy
ppSlide.Shapes.Paste.Select
ppSlide.Shapes(2).Height = _
    3 * (ppPres.PageSetup.SlideHeight / 5)
ppSlide.Shapes(2).Width = _
    5 * (ppPres.PageSetup.SlideWidth / 7)
ppSlide.Shapes(2).Left = _
    (ppPres.PageSetup.SlideWidth / 7)
ppSlide.Shapes(2).Top = _
    (ppPres.PageSetup.SlideHeight / 10)

End sub

Answer 1

Public ppApp As PowerPoint.Application
Public ppPres As PowerPoint.Presentation
Public ppSlide As PowerPoint.Slide

Assuming we're looking at a standard module, this is a red flag. Anyone, anywhere, can go and do this:

Set ppPres = Nothing

Or this:

Set ppApp = New PowerPoint.Application

...or otherwise shatter other procedures' assumptions. Variables' scope should be as limited as possible. These 3 public object variables are going to bite you, one day or another.

The ppApp application is never exited, which means more memory is consumed by the system after your macro runs, than before it did. And I wouldn't be surprised that the more you run it, the more memory it consumes. Why is that?

sub CreatePres()
Dim ppTextbox As PowerPoint.Shape
Dim slidesCount As Long
Set ppApp = New PowerPoint.Application

The procedure is doing much more than create pres; it creates a whole PowerPoint application instance, a new one every time it runs.

About object instantiation

Whoever creates an object, is responsible for that object's lifetime. When the object is no longer needed, the scope that created the object is also the scope that cleans it up.

Surely that Application class has a Quit method, or something similar - that method should be called before the procedure exits, in all exit points (i.e. even if an error occurs at any point).

My procedure would probably look something like this:

Option Explicit

Public Sub CreatePresentation()
    With New PowerPoint.Application
        Dim pres As PowerPoint.Presentation
        Set pres = .Presentations.Add
        CreateSlides pres
        SavePresentation pres
        pres.Close
        .Quit
    End With
End Sub

The procedure creates a presentation. An implementation detail of that, is that in order to create a presentation it first creates a PowerPoint app, but the outside world couldn't care less. Nor does it care about slides, or for a file name for the presentation (I suspect you're leaving PP open so that the user can tweak as needed and save when they see fit?) - these are separate concerns, there's a separate procedure at a lower abstraction level for that.

Kudos for beginning to split your code into separate procedures! At first it's normal not to be sure where or why split, but the more you familiarise yourself with the concept of abstraction, the easier it will become.

If you can look at a chunk of code and say "this chunk does XYZ", then chances are that it actually belongs in some DoXYZ procedure. Extracting the details of how slide2 is generated was a great first step.. but what about slide1? The part that deals with slide1 is mixing abstraction levels within the procedure: why do we need to know all the details about slide1 but slide2 gets its own dedicated procedure? The number of lines of code involved has nothing to do with the decision, only what's going on does.

Keep up the good work!

Rubberduck would have picked up a few issues:

• Option Explicit is not specified. That's dangerous, a mere typo and you have a bug, one that can be pretty tricky to locate.
• createPres is implicitly Public. Better make that explicit.
• slide2 is implicitly Public, but that one has no business being public. Should be Private
• Worksheets implicitly refers to the active workbook. May or may not be intended.
• Cells implicitly refers to the active worksheet. Here if you qualify the Range call with a worksheet object other than ActiveSheet, you would get a run-time error because of that.
• Call statement is obsolete and can safely be omitted (along with the parentheses around the arguments).
• Consider renaming parameter i. Naming is hard, but meaningful names are important. slidePosition would be a better name IMO... But does a procedure named slide2 need to know about where to insert the slide?
• Consider renaming procedure slide2 so that the name starts with a verb. Procedures do something: describe it.

Beyond that, I'd say use PascalCase for procedure names and keep camelCase for local variables [and parameters], and avoid Select and Activate where possible (use the object model instead).