Is there a better way to achieve this? (Potential performance problems with loops and delay???)

Hi everyone.  Here's the scenario:  I'm creating an application which allows users to select multiple clients and prefill PDFs (with info pulled from a datasource).  (User can also choose to complete multiple forms for multiple clients).  The following code IS working, but this application is going to be used exstensively and I'm concerned about performance.  Does anyone have any suggestions for improving the code below or is it as tight as it can be?  Thanks for any advice.



<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html>
<head>
<title>Forms Selector</title>
<LINK rel="stylesheet" type="text/css" href="/images/FRWS.css" title="public">
</head>
<body TOPMARGIN="0" LEFTMARGIN="0" MARGINWIDTH="0" MARGINHEIGHT="0">
<CFFLUSH>
<div id="Layer1" style="position:absolute; width:560px; height:115px; z-index:1; left: 30px; top: 0px; visibility: show">
    <OBJECT classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" codebase="https://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab##version=6,0,0,0" ID="loading" WIDTH="160" HEIGHT="50" ALIGN="">
         <PARAM NAME=movie VALUE/loading.swf">
         <PARAM NAME=quality VALUE=high>
         <PARAM NAME=wmode VALUE=transparent>
         <PARAM NAME=bgcolor VALUE=FFFFFF>  
         <EMBED src="/loading.swf" quality=high wmode=transparent bgcolor=FFFFFF  WIDTH="160" HEIGHT="50" swLiveConnect=true ID="loading" NAME="loading" ALIGN="" TYPE="application/x-shockwave-flash" PLUGINSPAGE="https://www.macromedia.com/go/getflashplayer"></EMBED>
    </OBJECT><div align="center"><h1>SHORT FLASH MOVIE DISPLAYING SOMETHING TO INDICATE CHOSEN FORMS ARE BEING COMPLETED</h1></div>
</div>
<CFFLUSH>


<!---### REMOVE ANY PREVIOUSLY GENERATED FORMS, THEY ARE NOT SAVED ON THE SERVER ###--->
<cfif NOT DirectoryExists("D:\ftpusers\#request.userID#\Forms\")>
     <cfdirectory action="CREATE" directory="D:\ftpusers\#request.userID#\Forms\">
</cfif>
<cfset VAR.FormPath = "D:\ftpusers\#request.userID#\Forms\">
<cfdirectory action="LIST" directory="#VAR.FormPath#" name="RemoveFiles">
<cfoutput query="RemoveFiles">
     <cfset FileName = "#VAR.FormPath##RemoveFiles.Name#">
     <cfif FileExists("#FileName#")>
          <cffile action="DELETE" file="#FileName#">
     </cfif>      
</cfoutput>
<cfset VAR.InvestorList="#FORM.ClientList#">
<!---### RETRIEVE LIST OF CLIENTS ###--->
<cfset Count="1">
<cfset VAR.List="">
<cfquery name="getClients" datasource="MyDatasource">
     SELECT      FIRST_NAME,LAST_NAME,TAX_ID_NUMERIC
     FROM       dbo.Clients
     WHERE      Client_ID IN (#VAR.InvestorList#)
</cfquery>
<cfset Count=#Count#>
<!---### LOOP CLIENTS AND RETRIEVE SELECTED FORMS FOR COMPLETION ###--->
<cfloop query="getClients">
<cfset FormsList="#FORM.FormsList"">
<cfset Count="#Count#">
<cfset VAR.List="#VAR.List#">    
<cfloop index="i" list="#FormsList#" delimiters=" ">
<!---### CREATE FDF FOR EACH CLIENT AND EACH FORM ###--->
<cfset cr = chr(13)>
<cfset lf = chr(10)>
<cfset crlf = cr & lf>
<cfset FDF_String = "%FDF-1.2" & cr & "%âãÏÓ" & crlf & "1 0 obj" & cr & "<< " & cr & "/FDF << /Fields [ ">
<cfset FDF_String = FDF_String & "<< /V (#getClient.TAX_ID_NUMERIC#)/T (SSN)>> ">
<cfset FDF_String = FDF_String & "<< /V (#getClient.FIRST_NAME#)/T (FirstName)>> ">
<cfset FDF_String = FDF_String & "<< /V (#getClient.LAST_NAME#)/T (LastName)>> ">
<cfset FDF_String = FDF_String & " " & cr & "] " & cr & "/F (#application.root#/forms/forms/#i#.pdf) >> " & cr & ">> " & cr>
<cfset FDF_String = FDF_String & "endobj" & cr & "trailer" & cr & "<<" & cr & "/Root 1 0 R " & cr & cr & ">>" & cr & "%%EOF" & cr>
<!---### WRITE FDF ###--->
<cffile action="WRITE"
        file="#VAR.FormPath##Count#.fdf"
        output="#FDF_String#"
        attributes="Normal"
        addnewline="Yes">
<!---### FILL PDF WITH CLIENT INFO ###--->
 <cfexecute
          name="D:\Httpdocs\field\eTouch\pdftk\pdftk.exe"
          arguments="D:\Httpdocs\field\FORMS\Forms\#i#.pdf fill_form #VAR.FormPath##Count#.fdf output #VAR.FormPath##Count#.pdf" timeout="100">
</cfexecute>    
<!---### CREATE THE LIST OF FORMS TO BE MERGED ###--->
<cfset VAR.List=#ListAppend(VAR.List, "#VAR.FormPath##Count#.pdf")#>
<cfset VAR.list = #replace(VAR.list, ",", " ", "ALL")#>
<cfset Count=#Count#+1>
</cfloop>    
<cfset Count=#Count#>
</cfloop>
<!---### MERGE ALL FORMS ###--->
 <cfexecute
          name="D:\HTTPDOCS\FIELD\forms\forms\pdftk.exe"
          arguments="#VAR.List# cat output #VAR.FormPath#merged.pdf" timeout="100">
</cfexecute>  
<!---### SET DELAY TO ALLOW FOR COMPLETION OF MERGE ###--->
<cfset delay = 1000>
<cfset sTime = gettickcount()>
<cfloop condition="gettickcount() lt sTime + delay">
<!---### IF THE MERGED FILE EXISTS OPEN MERGED PDF ###--->
     <cfif FileExists("#VAR.FormPath#merged.pdf")>
     <cfflush>
          <cfoutput>
               <script>
               window.location = '#application.root#users/#request.userID#/Forms/Merged.pdf';
               </script>
          </cfoutput>
     </cfif>
</cfloop>    


</body>
</html>
whaleykAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

mrichmonCommented:
Yes - you have a lot of lines that do nothing, lines that are inefficient and lines that have errors  (for example an opening # but no closing one after the variable)

Do not use # inside cf tags unless in double quotes - and only use double quotes when required

for example:
<cfset FormsList="#FORM.FormsList""> should be <cfset FormsList=FORM.FormsList>
<cfset Count=#Count#>  should be <cfset Count=Count>

but this is pointless as you are assigning the variable to itself - this line should be removed

There are a lot of lines that assign a variable to itself which is useless....
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
whaleykAuthor Commented:
Hi mrichmon, thanks for your feedback.  I actually know better regarding the # sounds inside the cf tags... (thanks for the reminder!!! Does it really make that much of a difference as far as performance???)  (I doctored the code up a little when i posted, so the missing # sign just came about as a result of that... the code wouldn't work if i actually was missing it.)

Not too sure what you mean about assigning the variable to itself being pointless.  The "count" is set outside the first loop and then needs to be "fed" within the second loop... i'll take a look tho and see if i can understand what you mean...

thanks very much for your time :-)
0
mrichmonCommented:

>>Does it really make that much of a difference as far as performance???

In most cases no - but in places where you have a large number of hits and/or efficiency is important because your code will be run a lot (like large loops) then yes.

But in most cases the difference is very minor and so visitors to your site wouldn't notice.


>>Not too sure what you mean about assigning the variable to itself being pointless.

This line:
<cfset Count = Count>  doesn't do anything.  If count was 1 it is still 1.

This line is useful
<cfset Count = Count + 1>


Now if your query has a variable called count then you really need to scope it such as:

<cfset Count = Queryname.Count>
0
Cloud Class® Course: CompTIA Cloud+

The CompTIA Cloud+ Basic training course will teach you about cloud concepts and models, data storage, networking, and network infrastructure.

whaleykAuthor Commented:
ha ha... sorry i missed the top line off :-)
<cfset Count="#RandRange(1000000,9999999)#">

Thanks for taking a look, i really appreciate your advice and happily award you the points for taking a look.

Kind Regards,
K
0
whaleykAuthor Commented:
Actually, i didn't miss it off, i just copied in the wrong template!!!!#@! :-)
0
mrichmonCommented:
That makes a difference :o)

But do this instead:

<cfset Count=RandRange(1000000,9999999)>

No double quotes and no # needed
0
whaleykAuthor Commented:
oops... yess... thanks again :-)
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Web Servers

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.