Code cleanup - PowerPoint COM

Get help with using AutoHotkey and its commands and hotkeys
Stavencross
Posts: 90
Joined: 24 May 2016, 16:42

Code cleanup - PowerPoint COM

12 May 2018, 13:08

Good afternoon! I've been working on this massive script for about 7 months now, and I'm refactoring all my code to be better/more elegant. I was hoping I could get some smarter people than I to take a look at my below function and help me understand how to write it better. The point of the script is that it helps a user (like myself) automate the building of a power point presentation and prep it to send to a client (I do this for 60 clients every month). Although this script does work, I'm just looking for a better way to write it. I have included a ZIP file containing the script and dependent files. Thank you for anyone that can offer any help!

Code: Select all

ServiceID = samesfordfd
accountType = ford


deckBuilder(ServiceID,accountType) {
	lastMonthFull = April
	thisMonthFull = May
	
	IfInString, serviceID, ford
	{
		AccountNameTitle = Ford ;We'll check if the presentation is for a ford client,or a lincoln client
		
	}
	else
	{
		AccountNameTitle = Lincoln
		
	}
	
	SplashTextOn,300,200,Building Deck, Grabbing your PDF and Building your deck, please wait!		
	sleep,1000
	Try
	{
		ppt := ComObjCreate("PowerPoint.Application")
		templatePath = %A_ScriptDir%\example.pptx ;Use the template I've included in the example ZIP
		ppt.Visible := true ;You can't use powerpoint COM while hidden for some reason
		pptPresentation  := ppt.Presentations.Open(templatePath)
		pptTotalSlides   := pptPresentation.Slides.Count
		firstReplacement = Digital Advertising Performance | Ford ;We're going to scan the titles of each slide for deckBuilder to work
		secondReplacement = Digital Advertising Performance | Lincoln
		thirdReplacement = Website Performance Insights
		firstAddition = Advertising Performance Overview
		secondAddition = Website Performance Overview
		thirdAddition = Website Digital Strategy
		firstImageTitle = %AccountNameTitle% Advertising Report
		secondImageTitle = %AccountNameTitle% Executive Summary Report
		thirdImageTitle = %AccountNameTitle% Quality Traffic Report
		firstInsights = %AccountNameTitle% Advertising Insights
		secondInsights = %AccountNameTitle% Analytics Insights
		thirdInsights = %AccountNameTitle% Web & Design Insights
		
	}
	catch exception
	{
		SplashTextOff
		msgbox % "There was an error opening powerpoint. Please try again or report it to your administrator.`nERR: " . exception.what . " | " . exception.line . " | " . exception.message . " | " . exception.extra  
		try
		{
			ppt.Quit()
		}
		catch e
		{
		}
		Exit
	}
	;We'll grab the PNGs I've included in the example zip so we can load them into the presentation later.
	execSummFilePath = %A_ScriptDir%\%ServiceID%_1.png
	trafficFilePath = %A_ScriptDir%\%ServiceID%_3.png
	advertisingFilePath = %A_ScriptDir%\%ServiceID%_5.png
	
	
	;We're checking if the files exist
	IfNotExist, %execSummFilePath%
	{
		ppt.Quit()
		SplashTextOff
		msgbox, We couldn't find your Executive Summary picture. Please run the downloader again or report it to your administrator.
		exit
	}
	IfNotExist, %trafficFilePath%
	{
		ppt.Quit()
		SplashTextOff
		msgbox, We couldn't find your Traffic Summary picture. Please run the downloader again or report it to your administrator.
		exit
	}
	ifNotExist,%advertisingFilePath%
	{
		ppt.Quit()
		SplashTextOff
		msgbox, We couldn't find your Advertising picture. Please run the downloader again or report it to your administrator.
		Exit
	}
	try
	{
		;slideCounter has to start at '1' or we get an index out of range error.
		slideCounter := 1
		loop %pptTotalSlides%
		{
			pptHasTitle := ppt.ActivePresentation.Slides(slideCounter).Shapes.HasTitle ;we'll check if there is a title on the slide
			dealerName = Sames Ford
			ppt.ActivePresentation.Slides(1).Shapes(1).Width:=900 ;We're going to change the name on the title slide
			ppt.ActivePresentation.Slides(1).Shapes(1).TextFrame.TextRange.Text := "" . dealerName . "" . " Performance Review"
			if(pptHasTitle= -1) ;if the slide has a title, it will return -1
			{
				
				pptDeleteSlide   := ppt.ActivePresentation
				pptSlideTitle    := Trim(ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text) ;grab the title
				
				if (pptSlideTitle == Trim(firstReplacement) || pptSlideTitle == Trim(secondReplacement) || pptSlideTitle == Trim(thirdReplacement))
				{
					pptDeleteSlide.Slides(slideCounter).Delete ;These slides should be removed so we'll delete them
					slideCounter -- ;the slide index numbers have changed, so decrement the slideCounter so we stay in the right spot
				}
				else
				{
					slideCounter ++
				}
			}
			Else
			{
				slideCounter ++
			}
			if (slideCounter = pptTotalSlides) ;we've reached the end of the presentation
			{
				Break
			}
		}
	}
	catch exception
	{
		SplashTextOff
		msgbox % "There was an error deleteing template slides from your presentation.Please try again or report it to your administrator ERR:" . exception.what . " | " . exception.line . " | " . exception.message . " | " . exception.extra  
		ppt.Quit()
		Exit
	}
	try
	{
				;Now that we've removed all the crap slides, we're going to add in our slides for our images and the user entered recommendations 
				;Notice that I'm essentailly repeating the entire loop again... there must be a better way?
		slideCounter = 1
		loop %pptTotalSlides%
		{
			pptHasTitle      := ppt.ActivePresentation.Slides(slideCounter).Shapes.HasTitle
			if(pptHasTitle= -1)
			{
				pptSlideTitle:= Trim(ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text)
				slideCounter ++
				if (pptSlideTitle = Trim(firstAddition)) ;if the slideTitle is the same as firstAddition, we're going to insert a new slide directly after it.
				{
					slideCounter --
					;I play with the slideCounter a lot, this could be done way better
					pptAddSlideTitle := ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange
					slideCounter ++
					pptPresentation.Slides.Add(slideCounter,2)
					ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text:=AccountNameTitle . " Advertising Report" ;lets change the title of our added slide
					
					slideCounter ++
					pptPresentation.Slides.Add(slideCounter,2)
					ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text:=AccountNameTitle . " Advertising Insights" ;the insights slide is where our users enter their recommendations
					
				}
				else if (pptSlideTitle = Trim(secondAddition))
				{
					pptAddSlideTitle := ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange
					pptPresentation.Slides.Add(slideCounter,2)
					ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text:=AccountNameTitle . " Executive Summary Report"
					
					slideCounter ++
					pptPresentation.Slides.Add(slideCounter,2)
					ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text:=AccountNameTitle . " Quality Traffic Report"
					
					slideCounter ++
					pptPresentation.Slides.Add(slideCounter,2)
					ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text:=AccountNameTitle . " Analytics Insights"
					
				}
				else if (pptSlideTitle = Trim(thirdAddition))
				{
					pptAddSlideTitle := ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange
					pptSlides := pptPresentation.Slides.Add(slideCounter,2)
					ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text:=AccountNameTitle . " Web & Design Insights"
					
					Break ;we're done with this loop, lets move on.
				}
			}
			else
			{
				slideCounter ++
			}
		}
	}
	catch exception
	{
		SplashTextOff
		msgbox % "There was an error adding slides to your presentation. Please try again or report it to your administrator ERR:" . exception.what . " | " . exception.line . " | " . exception.message . " | " . exception.extra  
			;ppt.Quit()
			;Exit
	}
	try
	{
	;Now that we've added all our slides & updated their titles, we're going to insert the PNG reports into the presentation. Again, you'll notice I'm completely repeating myself.
		slideCounter = 1
		loop %pptTotalSlides%
		{
			pptHasTitle      := ppt.ActivePresentation.Slides(slideCounter).Shapes.HasTitle
			if(pptHasTitle= -1)
			{
				pptSlideTitle:= Trim(ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text)
				pptShapes        := pptSlides.Shapes
				slideCounter ++
				if(pptSlideTitle = Trim(firstImageTitle))
				{
					slideCounter --
					pptPresentation.Slides(slideCounter).Shapes.AddPicture(advertisingFilePath, True, True, 250, 95,480,410) ;add the PNG, set the X/Y/width/Height
					slideCounter ++
					
				}
				else if (pptSlideTitle = Trim(secondImageTitle))
				{
					slideCounter --
					pptPresentation.Slides(slideCounter).Shapes.AddPicture(execSummFilePath, True, True, 250, 95,480,410)
					slideCounter ++
					
				}
				else if (pptSlideTitle = Trim(thirdImageTitle))
				{
					slideCounter --
					pptPresentation.Slides(slideCounter).Shapes.AddPicture(trafficFilePath, True, True, 250, 95,480,410)
					slideCounter ++
					
				}
			}
			else
			{
				slideCounter ++
			}
		}
	}
	catch exception
	{
		SplashTextOff
		ppt.Quit()
		msgbox % "There was an error adding images to the slides to your presentation.Please try again or report it to your administrator ERR:" . exception.what . " | " . exception.line . " | " . exception.message . " | " . exception.extra  
		Exit
	}
	SplashTextOff
	try
	{
	;Now that we've added our PNGs to the slides, we're going to let the user add their personal recommendation for their clients to the insights slides. A user will type the information and then click "YES" on the
	;MsgBox to advance powerpoint to the next slide, otherwise, they can hit "NO" to save their progress and exit, or "CANCEL" to exit. This feels like such a dirty way to capture user input, maybe there is a 
	;more graceful way?
		slideCounter = 1
		loop %pptTotalSlides%
		{
			pptSlide         := pptPresentation.Slides(slideCounter)
			pptHasTitle := ppt.ActivePresentation.Slides(slideCounter).Shapes.HasTitle
			if(pptHasTitle= -1)
			{
				pptSlideTitle:= Trim(ppt.ActivePresentation.Slides(slideCounter).Shapes.Title.TextFrame.TextRange.Text)
				if (pptSlideTitle = Trim(firstInsights))
				{
					advertisingSlidePosition := slideCounter
					slideCounter ++
					msgbox,3,Advertising Insights, Press YES to be taken to your Advertising Insights Slide. Enter your insights there and follow instructions in the following popup. Hit NO to save your changes as a PPT and exit. You will be able to make changes to the deck later. Hit CANCEL to Exit
					IfMsgBox Yes
					{
						pptSlide.Select
						slideCounter ++
					}
					IfMsgBox No
					{
					;Saves the presentation as PPTX. 11 is the COM constant.
						pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
						
						ppt.Quit()
						
						
						Exit
					}
					ifMsgBox Cancel
					{
						ppt.Quit()
						Exit
					}
				}
				else if (pptSlideTitle = Trim(secondInsights))
				{
					analyticsSlidePosition := slideCounter
					msgbox,3,Analytics Insights, Press YES to be taken to your Analytics Insights Slide.Enter your insights there and follow instructions in the following popup. Hit NO to save your changes as a PPT and exit. You will be able to make changes to the deck later. Hit CANCEL to Exit
					IfMsgBox Yes
					{
						pptSlide.Select
						slideCounter ++
					}
					IfMsgBox No
					{
						pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
						
						ppt.Quit()
						Exit
					}
					ifMsgBox Cancel
					{
						ppt.Quit()
						Exit
					}
				}
				else if (pptSlideTitle = Trim(thirdInsights))
				{
					webDesignSlidePosition := slideCounter
					msgbox,3,Web & Design Insights, Press YES to be taken to your Web & Design Insights Slide.Enter your insights there and follow instructions in the following popup.Hit NO to save your changes as a PPT and exit. You will be able to make changes to the deck later. Hit CANCEL to Exit
					IfMsgBox Yes
					{
						pptSlide.Select
						slideCounter ++
					}
					IfMsgBox No
					{
						pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
						
						ppt.Quit()
						Exit
					}
					ifMsgBox Cancel
					{
						ppt.Quit()
						Exit
					}
					Break
				}
				else
				{
					slideCounter ++
				}
			}
			else
			{
				slideCounter ++
			}
		}
	}
	Catch exception
	{
		SplashTextOff
		pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
		
		msgbox % "There was an error adding your insights to the slides to your presentation. Please try again or report it to your administrator ERR:" . exception.what . " | " . exception.line . " | " . exception.message . " | " . exception.extra  
		Exit
	}
	try
	{
		;We're done! we'll let our users interact with powerpoint and make any final changes, then we'll export the presentation to PDF automatically.
		MsgBox,3,Deck Builder, Automatic Deck Build completed.Please go through and double check your deck & make any other changes you'd like. Press YES to save your changes and export the Deck to a PDF. Press NO to save your changes and exit. Press Cancel to exit and disreguard changes.
		ifMsgBox Yes
		{
			dealerName = Sames Ford
			
			pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
			savePdf := pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "", ppSaveAsPDF := 32) ;We saved it as a PDF, 32 is the COM constant for PDF
			ppt.Quit()
		}
		ifMsgBox No
		{
			IniRead, dealerName, %accountINIPath%, AccountSettings, dealerName
			pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
			ppt.Quit()
			
		}
		ifMsgBox Cancel
		{
			ppt.Quit()
			Exit
		}
	}
	catch exception
	{
	;Just in case there was an error, we'll save it as PPTX so the user doesn't loose their changes.
		pptPresentation.SaveAs("""" . A_ScriptDir . dealerName . " " . thisMonthFull . "",ppSaveAsDefault := 11)
		ppt.Quit()
		SplashTextOff
		msgbox % "There was an error exporting your presentation to PDF. Please try again or report it to your administrator ERR:" . exception.what . " | " . exception.line . " | " . exception.message . " | " . exception.extra  
		Exit ;All Done!
	}
	Exit
}
User avatar
kczx3
Posts: 1502
Joined: 06 Oct 2015, 21:39

Re: Code cleanup - PowerPoint COM

12 May 2018, 15:40

I’m looking at this on my phone so bear with me.

Is that all one function besides the first two lines? I’d absolutely recommend breaking that up into pieces that are easier to follow. Or even make it a class.

Second, I would suggest getting used to using expression syntax. Always use the colon + equal operator and quotes for string literals. Just the first two things I thought of.
User avatar
kczx3
Posts: 1502
Joined: 06 Oct 2015, 21:39

Re: Code cleanup - PowerPoint COM

12 May 2018, 15:42

Also, to prep you for AHK v2, try to not use the compound if statements. Just use if with functions like InStr. And SplashText isn’t included in v2.
Stavencross
Posts: 90
Joined: 24 May 2016, 16:42

Re: Code cleanup - PowerPoint COM

12 May 2018, 16:14

Yes, right now it's all one function, that's kind of what I'm looking for, what parts of this could be broken up into additional functions?

Why shouldn't I use compound if statements?

This refactorization is the precurser to a full blown app rewrite where I do a lot more notifications via javascript & toast notifications in windows instead of splashtext, so that part isn't to big a deal. This function is one of the oldest ones in my app and its horrendously complex so I want to make sure before I start over again that it works much better.
swagfag
Posts: 5458
Joined: 11 Jan 2017, 17:59

Re: Code cleanup - PowerPoint COM

12 May 2018, 16:21

besides what kzxc3 has already mentioned:
do away with vars such as firstReplacement, thats a clear indicator that u need some kind of a data structure however simple it may be. use an array instead

stuff those message boxes and long ass strings somewhere far far away. its just additional noise when trying to figure out what the function actually does

copy-pasta segments indicate possible candidate for extract method

obviously do something about the loops
Stavencross
Posts: 90
Joined: 24 May 2016, 16:42

Re: Code cleanup - PowerPoint COM

12 May 2018, 17:33

swagfag wrote:besides what kzxc3 has already mentioned:
do away with vars such as firstReplacement, thats a clear indicator that u need some kind of a data structure however simple it may be. use an array instead
Ok, so I need to create an array, good suggestion!
stuff those message boxes and long ass strings somewhere far far away. its just additional noise when trying to figure out what the function actually does
Are you talking about when I'm posting a function for review?
copy-pasta segments indicate possible candidate for extract method
What?
obviously do something about the loops
What could be done about them?
User avatar
nnnik
Posts: 4497
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: Code cleanup - PowerPoint COM

16 May 2018, 02:45

Stavencross wrote:
copy-pasta segments indicate possible candidate for extract method
What?
He means that if you repeat specific code almost like it is copy pasted you should put it in a seperate function.
Recommends AHK Studio
Stavencross
Posts: 90
Joined: 24 May 2016, 16:42

Re: Code cleanup - PowerPoint COM

16 May 2018, 05:17

nnnik wrote:
Stavencross wrote:
copy-pasta segments indicate possible candidate for extract method
What?
He means that if you repeat specific code almost like it is copy pasted you should put it in a seperate function.
Ah!

Thank you for the insight!
User avatar
FanaticGuru
Posts: 1659
Joined: 30 Sep 2013, 22:25

Re: Code cleanup - PowerPoint COM

16 May 2018, 14:51

Here is two examples of techniques I would use.

Code: Select all

; init array
Replacements := {}
Replacements.Push("Digital Advertising Performance | Ford")
Replacements.Push("Digital Advertising Performance | Lincoln")
Replacements.Push("Website Performance Insights")

; loop array
for Index, Replacement in Replacements
	MsgBox % Index "`t" Replacement

; specific element
MsgBox % Replacements.2

; loop through slides
pptApp := ComObjActive("PowerPoint.Application")
for pptSlide in pptApp.ActivePresentation.Slides
	MsgBox % pptSlide.Name
One is just using an array to store related information in a way that it is easy to loop through and access.
The second is how to loop through the elements of a COM object, in this case loop through the slides of the active PowerPoint.

Also I would only use = for comparisons like if-statements and use := for assignments.
advertisingFilePath := A_ScriptDir "\" ServiceID "_5.png"
It makes it more clear what is going on and is the way of many languages and also future versions of AutoHotkey.

Also phase out commands like IfNotExist. These are deprecated techniques.
if FileExist(execSummFilePath)

FG
Hotkey Help - Help Dialog for Currently Running AHK Scripts

AHK Startup - Consolidate Multiply AHK Scripts with one Tray Icon

[Function] Timer - Create and Manage Timers

Return to “Ask For Help”

Who is online

Users browsing this forum: arczi_87, Boss55, Chunjee, Green Astronaut, inseption86, mikeyww, one1tick, XMCQCX and 60 guests