HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Complex spreadsheet script optimization

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptspreadsheetoptimizationcomplex

Problem

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 getValues).

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_

Solution

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' );

Code Snippets

var monitoringSheet = doc.getSheetByName("monitoring");
var toSheet = monitoringSheet;
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);
c4.setValue( 'blockxmlimport' );

Context

StackExchange Code Review Q#19486, answer score: 2

Revisions (0)

No revisions yet.