3
\$\begingroup\$

Can someone have a look at my script and maybe find something which can be optimized, as there shall be later 100 of it running.

First of all this script shall be triggered once per day to receive data from a website into a sheet called "getdata". Some fields of "getdata" are transferred to "blindcopy" in a certain order. After that the found data shall be put into a sheet called "monitoring" by deleting first row of this sheet and filling the data from "blindcopy" to the row after the last row of "monitoring". These data is referenced to other sheets and finally to the sheet "Output". In the sheet "Output" some cells are colored depending on their relative difference to other cells (first set color of all text in E7:E10 red and if any cell in this range is bigger than its corresponding cell in column D then its text color is set to green).

To not overflow the importXML command to the website on the beginning of the script the given URL is set to a cell and from there it is deleted at the end of the script again. This is because I do not want to call the importxml command everytime when I or others view the "Output" sheet, where none of the data is displayed coming directly via importxml, but it would be called, even if the data will not be displayed because the displayed data on "Output" is referencing to sheets where the importxml commands are executed (I hope I could explain this properly).

OK, the script is working fine no problems with that. But as I want to use this script in 100 different spreadsheets triggered on different times of the day once every day I think I might run into "exceeding CPU time" error. And before that I want to make sure that I have stripped the script down to the minimum (like I changed the getValue to getValue*s*).

It would be fine if someone could have a look into it, as there might be something to improve.

function myFunction() {
  // Open spreadsheet

  var docid = "some_long_strange_id_following_the_key=_in_the_URL_of_the_spreadsheet"
  var doc = SpreadsheetApp.openById(docid);
  var getsheet = doc.getSheetByName("getdata");
  // Get url from C1 and insert to multiple-referenced cell C4
  var cellreference = getsheet.getRange("C1");
  var url = cellreference.getValue();
  var cellreference = getsheet.getRange("C4");
  cellreference.setValue(url);
  // Transferring data from "blindcopy" to "monitoring"
  var getsheet = doc.getSheetByName("monitoring");
  getsheet.deleteRow(1)
  var sheetFrom = doc.getSheetByName("blindcopy");
  var sheetTo = doc.getSheetByName("monitoring");  
  var lastRow = sheetFrom.getLastRow();
  var cell = sheetFrom.getRange('A1').offset(lastRow, 0);
  var rowTo = sheetTo.getLastRow() + 1;
  for( var i = 1; i < 33; i++) {
     sheetTo.getRange(rowTo, i).setValues( sheetFrom.getRange(lastRow, i).getValues());
  }
  // Compare values1 to values2 for the coloring
  var s = doc.getSheetByName('Output');
  var values1Rule1 = s.getRange('E7:H10').getValues();
  var values2Rule1 = s.getRange('D7:D10').getValues();
  var matchList = [];
  for (var row in values1Rule1) {
    for (var col in values1Rule1) {
      for (var row2 in values2Rule1) {
        if (values1Rule1[row][col] >= values2Rule1[row][0]) 
          matchList.push(s.getRange('E7').offset(row, col, 1, 1).getA1Notation()); }}}
  // Turn values1 red first 
  s.getRange('E7:H10').setFontColor('RED');
  // Then only turn matches green
  for (var m in matchList)
    s.getRange(matchList[m]).setFontColor('GREEN');
  // delete C4 in "getdata" to avoid too much traffic to URL by importxml due to many lookups of "Output"
  var getsheet = doc.getSheetByName("getdata");
  var blubb = "blockxmlimport";
  var cellreference = getsheet.getRange("C4");
  cellreference.setValue(blubb);
}
\$\endgroup\$
4
  • \$\begingroup\$ How long does it take to run now and does it take up lots of resouces? \$\endgroup\$
    – DFord
    Commented Dec 11, 2012 at 12:52
  • \$\begingroup\$ How can I see how much resources it takes? \$\endgroup\$
    – Tassilo
    Commented Dec 11, 2012 at 22:16
  • \$\begingroup\$ Script runs round about 10 seconds \$\endgroup\$
    – Tassilo
    Commented Dec 11, 2012 at 22:57
  • \$\begingroup\$ for (var row in values1Rule1) { for (var col in values1Rule1) { for (var row2 in values2Rule1) { looks fishy \$\endgroup\$
    – megawac
    Commented Jan 21, 2014 at 2:35

1 Answer 1

2
\$\begingroup\$

From my research, the slowest part of your script is by far SpreadsheetApp.openById(docid), there is nothing much you can do about that. I am not sure how this script will behave once you have a 100 instances of it running, but my prediction is that it will be bad..

Furthermore from a once over:

  • You never use cell in var cell = sheetFrom.getRange('A1').offset(lastRow, 0);
  • You never use row2 in for (var row2 in values2Rule1) {, I dont think you need it
  • You only have to declare cellreference and getsheet once
  • You should consider lowerCamelCasing: getsheet -> getSheet, cellreference -> cellRefeference
  • getSheet is a terrible name, you should name the variable of the sheet:

    var monitoringSheet = doc.getSheetByName("monitoring");
    
  • You call doc.getSheetByName("monitoring") twice, you could simply:

    var toSheet = monitoringSheet;  
    
  • Instead of reading one cell value and then assigning it to another, you could simply use copyTo:

    var dataSheet = doc.getSheetByName("getdata");
    // Get url from C1 and insert to multiple-referenced cell C4
    var c1 = getsheet.getRange("C1"),
    var c4 = getsheet.getRange("C4");
    c1.copyTo(c4);
    
  • At the end you already had a reference to C4, so you could simply:

    c4.setValue( 'blockxmlimport' );
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.