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
    • 11 mths 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
      • 11 mths 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
      • 11 mths 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
      • 11 mths 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
      • 11 mths 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
    • 11 mths 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
    • 11 mths ago
    • Reported - view

    I knew it could be better written!

    Thanks guys 👍

Content aside

  • Status Answered
  • 11 mths agoLast active
  • 7Replies
  • 72Views
  • 4 Following