Skip navigation
KuddRoww
Currently Being Moderated

My First Script - Critique Wanted!

Jan 11, 2012 10:35 AM

I just finished "my" first script (I've had a lot of help along the way) and I've inserted it below as a quotation for quick reference.

 

The first question I have is whether it is possible to open InDesign files in the "background". With the script below I feel that if I run it in the "background" or hidden, so that each subsequent file doesn't have to be "graphically" processed I could save some time. How would I do this? I thought by not specifying a window for the InDesign file to opened in the file would simply remain in the background.

 

The second question I have is maybe a more general programming question. How do I open each file one at a time and then process the file and close it? The reason I ask is because for the script below if it is run on say three items it takes very little time, however when it is run on a dozen items it seems to take much longer, I'm assuming it's because of the resources consumed by having so many files open at the same time. I know it has to do with loops and I've done some research on it but a concrete example of how to effectively "apply a loop on a loop" (haha clearly not the correct terminology).

 

The final question I have specifically relates to the script below. If you were to write this script how would you have done it differently? I have very little scripting/programming experience and would be very interested in seeing the "proper" way to go about this.

 

To summarize I wanted this script to:

 

Open the InDesign files, export them as interactive PDFs with facing pages disabled and then close the files without saving. My preference would be for the process to be expandable to a dataset of a few hundred files without loss of stability, doing this quickly is an added bonus however the elegance of such a solution would have to strong factor in simplicity.

 

Thank-you in advance for all of your help and attention in this matter,

 

Kurtis

//Explain the purpose of the script.

alert("The purpose of this script is to export a selection of InDesign files into single-page view interactive PDF files.")

//Set the folder of InDesign files which are to be converted into Interactive PDFs

var myFolder = Folder.selectDialog("Please select a directory of InDesign files which you would like converted into single-view interactive PDFs.");

//Index the files located in the selected folder

var files = myFolder.getFiles ("*.indd");

var outFolder = Folder.selectDialog("Now please select a directory in which you would like to save the interactive PDFs to.");

//User interface interaction on alert only

app.scriptPreferences.userInteractionLevel=UserInteractionLevels.inter actWithAlerts;

//Go through directory of files chosen by user and apply document setup change

for(i = 0, l = files.length; i <l; i++) {

         myFile = app.open(files[i], true)

with(app.activeDocument.documentPreferences){

    facingPages = false;

with(app.interactivePDFExportPreferences)

   viewPDF = false;

}

myFile.exportFile(ExportFormat.interactivePDF, File(outFolder + "/" + myFile.name.replace(/\.indd$/,"") +".pdf"), false);

 

}

for(myCounter = app.documents.length; myCounter > 0; myCounter--){

app.documents.item(myCounter-1).close(SaveOptions.NO);}

 

alert("Interactive PDFs have been generated.")

 

//Close all open files

 

 
Replies
  • Currently Being Moderated
    Jan 11, 2012 12:44 PM   in reply to KuddRoww

    Nice job! You are well on your way.

     

    The closest thing to opening your document in the background would be to change this line:

     

    myFile = app.open(files[i], true)

     

    to this:

     

    myFile = app.open(files[i], false)

     

    That would open the file without a window.

     

    As for the second question. Instead of that last loop put after this line:

     

    myFile.exportFile(ExportFormat.interactivePDF, File(outFolder + "/" + myFile.name.replace(/\.indd$/,"") +".pdf"), false);

     

    this:

     

    myFile.close(SaveOptions.NO);

     
    |
    Mark as:
  • Currently Being Moderated
    Jan 11, 2012 1:32 PM   in reply to KuddRoww

    Watch out with changing the script interaction level. It is still at this setting when the script finishes. Oh, and I think your script won't show any dialogs anyway so you can leave it unchanged to begin with.

     
    |
    Mark as:
  • Currently Being Moderated
    Jan 11, 2012 1:37 PM   in reply to [Jongware]

    Unless the doc has missing fonts or missing links...

     

    Or does interactWithAlerts not show those?

     
    |
    Mark as:
  • Currently Being Moderated
    Jan 11, 2012 2:05 PM   in reply to KuddRoww

    As jongware said make sure to add this to the end of your script:

     

    app.scriptPreferences.userInteractionLevel = UserInteractionLevels.INTERACT_WITH_ALL;

     
    |
    Mark as:
  • John Hawkinson
    5,572 posts
    Jun 25, 2009
    Currently Being Moderated
    Jan 12, 2012 2:16 PM   in reply to KuddRoww

    Hi, Kurtis!

     

    Congrats on your first script. Looks like you got your basic questions answered, so I'll turn towards the critque.

    With this forum software, when posting scripts, it really helps to use the Advanced Editor and click >> Syntax Highlighting > Java to post your script. It makes it a lot more readable and reduces the liklihood of things going badly wrong with formatting and line breaks and whatnot.

     

    So, if I were writing your script, some things I would do differently:

     

    . Wrap it all in a function. That will help you out as your program gets bigger and more complicated, and you want to reuse code. It also helps you distinguish between local and global variables. And usually you want to avoid using global variables at all, so this is important. So your script would go like this:

     

    function main() {
         alert("The purpose of this script is to export a selection of InDesign files into single-page view interactive PDF files.")
        ...
        alert("Interactive PDFs have been generated.")
    }
     
    main();
    

     

    . Comments in code are good. But a comment that is completely obvious from reading the code is pointless.

    Compare:

     

    var sum=a+b; // Add a to b
    

     

    and

     

    var sum=a+b; // Add box height to circle height
    

     

    The first case is completely useless. You're better off without the comment.

    And actually, what you really should do is something like:

     

    var sum=boxHeight + circleHeight;
    

     

    Comments are useful, but many of yours really are like the first category. They just serve as clutter and run the risk of getting out of date when things change. THey also make the script a bit longer than it really is...

     

    . Use of var.

    Javascript has a lot of language design problems, becuse it was written in a few weeks when most languages were written over years. They made some mistakes. Several of them relate to variables and variable scoping.

     

    Unlike most languages, which have block scope, Javascript has function scope for variables. No matter where in a function you declare your variables, they are accessible throughout the function, and scoped to the whole function.

     

    As a result, it's good Javascript sstyle to declare variables at the top of the function, and only at the top of the function. In fact, Crockford would encourage you to use only one 'var' statement as well, and I think he is probably right.

     

    So that turns your function into:

    function main() {
        var
            myFolder, files, outFolder, i, myFile, myCounter;
    ....
    

     

    . My users are always frustrated by unnecessary prompts. I would combine your first alert() and selectDialog() and drop the alert.

     

    . You might want to check the return values of functions, like selectDialog(). What happens if the user clicks Cancel?

     

    . You managed not to declare some of your variables, making them into global variables (i, myCounter, etc.) Be careful with that.

     

    . Generally speaking, "with" is considered harmful, because while it seems convenient, it is very hard to predict what it actually does. Replace:

     

    with(app.activeDocument.documentPreferences){
        facingPages = false;
    

     

    with:

    app.activeDocument.documentPreferences.facingPages = false;
    

     

    or perhaps:

    var doc = app.activeDocument;
    ...
    doc.documentPreferences.facingPages = false;
    

     

    or even:

    var doc = app.activeDocument,
        docPrefs = doc.documentPreferences;
    ...
    docPrefs.facingPages = false;
    

     

    . Javascript lets you omit semicolons sometimes. Don't do it! It's a bad habit, and there are places where the interpreter will stuff them in and cause you serious problems. Always include them:

    for(i = 0, l = files.length; i <l; i++) {
             myFile = app.open(files[i], true)
    

     

    is asking for trouble.

     

    . This is too hard to read:

     

    for(myCounter = app.documents.length; myCounter > 0; myCounter--){
    app.documents.item(myCounter-1).close(SaveOptions.NO);}
    

     

    Don't offset your loop variable inside the body of the loop. Just build it correctly

    from the start (also, use whitespace):

     

    for (myCounter = app.documents.length-1; myCounter => 0; myCounter--) {
        app.documents.item(myCounter).close(SaveOptions.NO);
    }
    

     

    I think that's it for now.  Hope this is helpful.

     
    |
    Mark as:
  • Currently Being Moderated
    Jan 12, 2012 2:49 PM   in reply to John Hawkinson

    (Adding to John's very constructive remarks:)

     

    for (myCounter = app.documents.length-1; myCounter => 0; myCounter--) {
        app.documents.item(myCounter).close(SaveOptions.NO);
    }
    

     

    I think that's it for now.  Hope this is helpful.

     

    Generally speaking, there is no reason for all loops to run backwards. It's only when interacting with InDesign's text model when this is virutally mandatory, because "earlier" changes in the text may invalidate "later" locally stored references. So the above could be written a bit clearer as

     

    for (myCounter = 0; myCounter < app.documents.length; myCounter++)
    {
       app.documents.item(myCounter).close (SaveOptions.NO);
    }
    

     

    (Notice John's and my different preferences for where to put the curly braces It's purely a matter of personal preferences, so don't fret about that one.)

     

    Technically, your own attempt -- and John's improvement quoted above -- has a potentially very nasty feature/bug. You open a list of files and process them one by one; then, you close all of the opened documents without saving! Moving the .close function into the processing loop, as Fred Goldman suggests, avoids this.

     

    FYI, if you do need to have a stack of documents opened all at once -- say, to process an entire Book worth of files --, you can use a custom array to store the opened documents in like this:

     

    openedDocs = []; // or 'new Array' but the result is the same
    for(i = 0; i < files.length; i++)
    {
       openedDocs.push(app.open(files[i], false));
    }
    //... process file array here ...
    // Close all files
    while (openedDocs.length)
       openedDocs.pop().close (SaveOptions.NO);
    

     

    (When using a construction like the above: .pop works in reverse, so the last item pushed will be the first item popped. In this case it's inconsequential, but I have encountered situations -- i.e., "bugs" -- where it does matter.)

     
    |
    Mark as:
  • John Hawkinson
    5,572 posts
    Jun 25, 2009
    Currently Being Moderated
    Jan 12, 2012 4:05 PM   in reply to [Jongware]

    Let's get in some friendly rivalry here!

     

     

    Generally speaking, there is no reason for all loops to run backwards. It's only when interacting with InDesign's text model when this is virutally mandatory, because "earlier" changes in the text may invalidate "later" locally stored references. So the above could be written a bit clearer

    Well, there is a school of thought (which I don't subscribe to) that says: Since sometimes loops must run from largest to smallest, and most of the time they don't matter, it's a good idea to try to do all your loops the same way. Thus concluding that all loops should run from largest to smallest where feasible. (It's not "backwards!")

     

    (Notice John's and my different preferences for where to put the curly braces It's purely a matter of personal preferences, so don't fret about that one.)

    It's actually quite a bit stronger than that.

    A lot of programmers work in, on, or around projects that enforce a strict style. There are many styles out there.

    Most of them are just wrong. A few are OK.

    http://en.wikipedia.org/wiki/Indent_style has a pretty reasonable discussion of them.

     

     

    while (openedDocs.length)
       openedDocs.pop().close (SaveOptions.NO);

     

    There are two issues with this block. The first is that it's pretty well-accepted that you should never omit the optional { } braces that define a block for a statement that can take them, because of the danger of someone editing the code like this:

     

    while (openedDocs.length)
       doSomethingFirst();
       openedDocs.pop().close (SaveOptions.NO);
    

     

    and of course, that will be parsed as:

     

    while (openedDocs.length) {
       doSomethingFirst();
    }
    openedDocs.pop().close (SaveOptions.NO);
    

    but you really meant:

     

    while (openedDocs.length) {
       doSomethingFirst();
       openedDocs.pop().close (SaveOptions.NO);
    }
    

    So instead, even for 1-line, always write:

     

    while (openedDocs.length) {
       openedDocs.pop().close (SaveOptions.NO);
    }
    

    even though it looks redundant.

     

    Also, combining statements in a clever fashion like this is...clever, but it tends to confuse non-expert programmers, and that's most of whom will be reading your scripting code. So, much better to stick it in a temporary variable for the sake of clarity:

     

    var adoc;
    ...
    while (openedDocs.length) {
        adoc = openedDocs.pop();
        adoc.close(SaveOptions.NO);
    }
    

     

    Though it might be better to just write it as a for loop, since that's how most people understand Array iteration best.

     

    Edit: Fixed some blank lines that snuck in (Jive).

     
    |
    Mark as:

More Like This

  • Retrieving data ...

Bookmarked By (0)

Answers + Points = Status

  • 10 points awarded for Correct Answers
  • 5 points awarded for Helpful Answers
  • 10,000+ points
  • 1,001-10,000 points
  • 501-1,000 points
  • 5-500 points