0

Improve my script?

I have a "Print" button script that Works without issue!

The idea is that if a customer quote contains product images in it, then the appropriate printout format is selected to include or not include images as the case may be.  At the same time the quote image is included in a thumbnail image field on the quote screen to show the use that the correct format has been generated

However, it took me a while to get it to this state as I previously I was constantly getting void errors - probably because i was having to many nests in the script.

So what i would now like to do is rewrite only better ! I think the the test for each image is messy and the bulk of the script repeats itself after the print format is chosen for myP. (ie the last 6 lines in each section)

I'm so used the if then else but wondering if someone can suggest a better/tidier/more concise way to express the same job ? 

 

if 'Draft!' = true then
                alert("Draft Quote Only Saved ! Turn Off Draft Flag To Print Final Quote!")
else
                QuoteImage := null; 
                if QImage1 != null or QImage2 != null or QImage3 != null or QImage4 != null then
                                let myP := "118PrintQuotePics";
                                importFile(this, printAndSaveRecord(this, myP), "SQ " + QteNum + ".pdf");
                                let myPdf := printAndSaveRecord(this, myP);
                                let myName := "SQ " + QteNum + ".pdf";
                                importFile(this, myPdf, myName);
                                waitForSync();
                                QuoteImage := myName
                else
                                let myP := "118PrintQuote";
                                importFile(this, printAndSaveRecord(this, myP), "SQ " + QteNum + ".pdf");
                                let myPdf := printAndSaveRecord(this, myP);
                                let myName := "SQ " + QteNum + ".pdf";
                                importFile(this, myPdf, myName);
                                waitForSync();
                                QuoteImage := myName
                end
end
 

7 replies

null
    • John_Halls
    • 1 yr ago
    • Reported - view

    Hi Mel

    How about

    if 'Draft!' then
   alert("Draft Quote Only Saved ! Turn Off Draft Flag To Print Final Quote!")
else
   QuoteImage := null;
   if QImage1 = null and QImage2 = null and QImage3 = null and QImage4 = null then
      let myP := "118PrintQuote"
   else
      let myP := "118PrintQuotePics"
   end
   importFile(this, printAndSaveRecord(this, myP), "SQ " + QteNum + ".pdf");
   let myPdf := printAndSaveRecord(this, myP);
   let myName := "SQ " + QteNum + ".pdf";
   importFile(this, myPdf, myName);
   waitForSync();
   QuoteImage := myName
end

    If 'Draft!' then should work without the = true

    I've swapped the not equal / or's into equal / and's as they always read better to my eye

    The duplicated code is pulled out so that it's only needed once.

    I've not changed it but couldn't see why QuoteImage was set to null, not used, and then set to myName

    Regards John

      • Mel_Charles
      • 1 yr ago
      • Reported - view

       Cheers John

      Looks like i'm not that far adrift. The reason for making Quoteimage := null in the first place is simple. If the user goes on to make price amendment(s) after the report has printed and saved to that field, there is no visual clue to the user that the image has updated it. Thus by having it clear after the button is pressed the user sees the image field empty and redraw so to speak...

      Your code looks good - hmmm but it errors just after the first end (ninox don't like it.) if you remove this end then the procedure does not work properly.

      • John_Halls
      • 1 yr ago
      • Reported - view

       Try a semi-colon after the middle 'end'

      if 'Draft!' then
   alert("Draft Quote Only Saved ! Turn Off Draft Flag To Print Final Quote!")
else
   QuoteImage := null;
   if QImage1 = null and QImage2 = null and QImage3 = null and QImage4 = null then
      let myP := "118PrintQuote"
   else
      let myP := "118PrintQuotePics"
   end;
   importFile(this, printAndSaveRecord(this, myP), "SQ " + QteNum + ".pdf");
   let myPdf := printAndSaveRecord(this, myP);
   let myName := "SQ " + QteNum + ".pdf";
   importFile(this, myPdf, myName);
   waitForSync();
   QuoteImage := myName
end

      Regards John

      • Mel_Charles
      • 1 yr ago
      • Reported - view

       Nargh - don't like it

      no worries as said the script is working - just me trying to make it cleaner

      • John_Halls
      • 1 yr ago
      • Reported - view

      Mel Charles This is because the let myP statement is within If.. then.. else. Fabio's revision corrects for this.

    • Sviluppatore Ninox
    • Fabio
    • 1 yr ago
    • Reported - view

    Hi  ,  .

    You could try this:

    if 'Draft!' then
    alert("Draft Quote Only Saved ! Turn Off Draft Flag To Print Final Quote!")
else
    QuoteImage := null;
    let myP := if QImage1 or QImage2 or QImage3 or QImage4 then
            "118PrintQuotePics"
        else
            "118PrintQuote"
        end;
    let myPdf := printAndSaveRecord(this, myP);
    let myName := "SQ " + QteNum + ".pdf";
    importFile(this, myPdf, myName);
    QuoteImage := myName
end

    Some considerations:

    • waitForSync() returns a boolean, so without a conditional check it makes no sense put there;
    • you only need to use the importFile() and printAndSaveRecord() functions once.

    Ciao

       Fabio

    • Mel_Charles
    • 1 yr ago
    • Reported - view

    I knew it could be better written!

    Thanks guys 👍

Login to reply

Content aside

  • Status Answered
  • 1 yr agoLast active
  • 7Replies
  • 72Views
  • 4 Following