Set Dates on Spreadsheet Rows

Question

I'm trying to get this to run faster, even marginally so. Right now to update 470 rows it takes about 90 seconds. It works, but I know there are better ways. I understand calls to Spreadsheet API are slow, and batching getRanges and setValues would speed things up, but doesn't seem obvious given the brief bit I know.

function setDates() {
var app = SpreadsheetApp;
var ssData = app.getActiveSpreadsheet().getSheetByName("Data");
//Calculates due dates based on entered dates in column D
 for (var i=4; i<=ssData.getLastRow(); i++) { //cycles through all 18 pilots
 for(var j=0; j<25; j++) { //cycles through an individuals' 25 training items
 var dateCompleted = new Date(ssData.getRange(i+j,4).getValue()); //date of training completed
 var interval = ssData.getRange(i+j,5).getValue(); //interval in months between date complete and date due
 var monthCompleted = dateCompleted.getMonth();
 var yearCompleted = dateCompleted.getFullYear();
 if (!isNaN(parseFloat(yearCompleted)) && !isNaN(interval)) { //checks to see if vars year and interval are numbers
 var dateDue = new Date(dateCompleted.setMonth((monthCompleted + interval))); //date of training due
 var yearDue = dateDue.getFullYear();
 var monthDue = dateDue.getMonth();
 dateDue = new Date(yearDue, monthDue+1, 0);
 ssData.getRange(i+j,6).setValue(new Date(dateDue)); //sets dateDue in column F
 }
 }
 i = i + 25;
 }
}

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate.
• Colin: I'm getting really bummed that in posting this twice on the forums, I've received no help but instead handfulls of comments about what I did wrong to post the question. I'm beginning to think this is not the area in which to find my answer....
• Sᴀᴍ Onᴇᴌᴀ: It looks like the SO post only had an image of the code and here the code is embedded so good job with that. It looks like now you have an answer- hopefully you find it helpful!

Answer 1

General review

Generally the code is OK and problems are due to lack of experience rather than bad style or habit.

I am unfamiliar with SpreadsheetApp so you should wait in case someone that is adds another answer before you accept this answer, if so inclined.

Rather than submit this answer I took a quick look at the DOC for SpreadsheetApp (I hope I got the correct one) and added to this answer at bottom under heading Performance.

Review points

• Take care with indentation. all code blocks should be indented (If indentation mess is due to snippets poor formatting ignore this line)

• When using var declare them at the top of the function.

• Use constants const for variables that do not change.

• Use i, j only for indexes or counters, if you are accessing coordinates, row, column, or x, y use names to match the axis name.

  In this case it looks like you are only indexing by row index. In the rewrite I use the variable name row. Then a counter called i to count 25 and allow for the skipped row every 26 rows.

  BTW When using i, j in loops the convention is i for the inner loop and j is the outer.

• Don't calculate the same value many times. Eg you calculate i + j 3 times.

• Avoid single use variables.

• Use while loops in preference to for loops especially if you are incrementing the counter manually in the loops block.

  While loops are generally syntactically cleaner and are less complex under the hood (if you use let to declare the counter).

  Note that when using while loops it is easy to forget to add the incrementation so take care to not forget to increment / decrement

• Use arithmetic assignment operators in this case += (Addition assignment) i += 25 rather than i = i + 25;

• The is no need to create a new Date object every time you make a change.

Date object

The Date objects call Date.getFullYear always returns a Number as an integer.

There is no need to use parseFloat and the result of !isNaN(parseFloat(yearCompleted)) will always be true and is thus not required.

Further the variable yearCompleted is only used in the unneeded predicate and as such is not required at all.

To set the date to the first day of the month you don't need to use year, month + 1, 0 just set it to the 1st year, month, 1

The inner if statement

It is unclear if the call to get interval = ssData.getRange(i + j, 5).getValue() will return a number or not.

Because the use of monthCompleted is dependent on the valid numerosity of interval you can combine the two in one expression. However I suspect that interval will always be a number and as such there is no need for the isNaN test.

Rewrite

Using the above points the rewrite greatly reduces the code complexity. Without data there is no way to know if there is a need to vet the interval value. I added the test but it may not be needed.

function setDates() {
 var row = 4, i;
 const data = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Data");
 while (row < data.getLastRow()) {
 i = 25;
 while (i--) {
 const interval = data.getRange(row, 5).getValue();
 if (!isNaN(interval)) {
 const completed = new Date(data.getRange(row, 4).getValue()); 
 completed.setMonth(interval + completed.getMonth());
 completed.setDate(1);
 data.getRange(row, 6).setValue(completed); 
 }
 row ++;
 }
 row ++;
 }
}

Performance

I have taken a quick look at the SpreadsheetApp docs and thus will address the performance problems you are concerned about.

The problem I think is the use of getRange(row, column) which you use to get each cell.

Rather...

• Use getRange(row, column, numRows, numColumns) to get all the rows and columns in one call before you iterate the cells.

• Then get an array of values from the range using range.getValues() make the modifications needed.

• Then use range.setvalues() to set the changes made

This may get a significant performance increase.

Rewrite based on Docs

As I have no data or access to the API the rewrite is only a guess based on a quick read of the DOC's

The only problem I can foresee is why you skip every 26th row and why you start from the 5th row (index as row 4)

function setDates() {
 var rowIdx = 0;
 const sheet = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Data");
 const range = sheet.getRange(4, 4, sheet.getLastRow() - 4, 3);
 const data = range.getValues(); // as 2D array of rows by columns eg...
 // data[0][0] is row 4 column 4
 // data[10][2] is row 14 column 6
 for (const row of data) {
 if (rowIdx > 0 && (rowIdx % 26)) { // skip every 26th row
 const interval = row[1];
 if (!isNaN(interval)) {
 row[2] = new Date(row[0]);
 row[2].setMonth(interval + row[2].getMonth());
 row[2].setDate(1);
 }
 }
 rowIdx++;
 }
 range.setValues(data);
}

• Colin: Wow, thanks @Blindman67, I will give these suggestions and try and let you know. Many, many thanks!