Incrementor not updating after iteration of loop

Viewed 94

I'm new to stackover flow and coding in general. I've put together a script that pulls information from array that pulls the information from the main sheet. The scripts pulls the type of sheet to pull from templates and fills it out based on the array data. It works fine with the first row of data but my variables don't update after an increment command so it gets stuck on the first entry. Apologies if I'm explaining it poorly. Function: 1.Pulls room type based on row 4 of Arraydata 2.Renames it based on row 4 of array data 3.Fills in Data from row 4 of Array data 4.Increments variable of row so that this can run the script again on row 5 until there is no more array data.

I have replaced the actual sheet ID for security

function createautomation() {
do{
var roomlist = SpreadsheetApp.getActiveSheet().getRange("Room List!A1:G100").getValues();
var i = 4
var ss = SpreadsheetApp.getActiveSpreadsheet();
var commissioning_template = SpreadsheetApp.openById('ID')
var roomtype = roomlist[i][5]
var copyroom = (`Copy of ${roomtype}`)
var ss = SpreadsheetApp.getActiveSpreadsheet();
var first = ss.getSheetByName(copyroom);
var roomname = roomlist[i][1]
var tab = (`${roomname} - ${roomtype}`)
var second = ss.getSheetByName(tab)
var room1 = (`${tab}!B1`)
var roomnum = (`${tab}!B2`)
var floor = (`${tab}!D2`)
var address = (`${tab}!B3`)
var siteName = (`${tab}!D3`)
var siteCode = (`${tab}!E3`)
var sourcenum = roomlist[i][0]
var sourceflr = roomlist [i][4];
//Copy The spreadsheet
commissioning_template.getSheetByName(roomtype).copyTo(ss);
//Rename Sheet
ss.getSheetByName(copyroom).setName(tab);
//Set Room Name
ss.getRange(room1).setValue(roomname);
//Set Room Number
ss.getRange(roomnum).setValue(sourcenum);
//Set Floor
ss.getRange(floor).setValue(sourceflr);
//Set Address
ss.getRange(address).setValue(roomlist[1][2]);
//Set Site Name
ss.getRange(siteName).setValue(roomlist[0][2]);
//Set Site Code
ss.getRange(siteCode).setValue(roomlist[2][2])
//increment the value of i
i++;
console.log(i)}
while(roomname !="")
}
1 Answers

Comments from Marc and Cooper regarding your function is actually right. Given those comments, I tried to improve your script. This will improve the performance but given the data you are processing, I'm not so sure if it will affect the performance that much but it is a good practice to do the following as this approach in large data sets is definitely better.

These are the things that you can improve so kindly check if you have the time.

function createautomation() {
  var ss = SpreadsheetApp.getActiveSpreadsheet();

  var roomlist = ss.getSheetByName("Room List") // get sheet separately from range
                   .getRange('A1:G100')         // you can also use A1:G if the column can grow past 100 and not sure until where           
                   .getValues()
                   .filter(row => row[1]);      // only include rows that have values on second column

  // pull all variables that are static in the loop
  var address = roomlist[1][2];
  var sitename = roomlist[0][2];
  var sitecode = roomlist[2][2];
  // separated tab as it varies between rows
  // combined the ranges that are in the same column
  var room1_roomnum_address = 'B1:B3';
  var floor_siteName = 'D2:D3';
  var siteCode = 'E3';
 
  var commissioning_template = SpreadsheetApp.openById('ID');

  // for all data to start at index 4, remove index 0 to 3 of roomlist
  roomlist.splice(0, 4);

  // loop all data remaining (this should start at 4th index which is 5th row of sheets data)
  roomlist.forEach(row => {
    // assign columns A, B, E and F each row to the following variables
    // other columns that weren't used are assigned to colC, colD, and colG
    var [sourcenum, roomname, colC, colD, sourceflr, roomtype, colG] = row;
    var copyroom = `Copy of ${roomtype}`;
    var tab = `${roomname} - ${copyroom}`;
    var tabSheet = ss.getSheetByName(tab);

    commissioning_template.getSheetByName(roomtype).copyTo(ss);
    ss.getSheetByName(copyroom).setName(tab);

    // set room1, roomnum and address at the same time
    tabSheet.getRange(room1_roomnum_address).setValues([[roomname],[sourcenum],[address]]);
    // set floor and siteName
    tabSheet.getRange(floor_siteName).setValues([[sourceflr],[sitename]]);
    // set siteCode
    tabSheet.getRange(siteCode).setValue(sitecode);
  });
}

Note:

  • It's a personal preference to separate the tab name with range but it is easier for me to read the code when they are separated.
  • Since you are checking your do loop if roomname is blank and stop there, I used filter after getValues to remove rows with blank column B values. Although this will still get the rows with values in column B after a blank row has been seen. I assume your data is continuous and has no blank rows in between so this will work. If not, comment below so I can modify this.
  • splice will permanently remove the rows indicated. So given that I chose to remove first 4 rows, save every cell value you need to variables before the splice command. Alternatively, you can modify this approach and use slice instead to not delete it and use forEach afterwards but I prefer this approach if I have no use on the rows above.
  • Currently, we are looping A5:G100 in the forEach and row contains all the values. I prefer using forEach as I won't need to mind about the iterator since it will iterate the array itself without any issues.
  • As what Cooper has mentioned, it is very slow to read and write in each loop. So I tried to combine multiple setValues into setValues. This accepts a 2D array so make sure to properly format your data before writing.
    • If you write data into a single column, array should be in this form: [[A1], [A2], [A3]]
    • If you write data into a single row, array should be in this form: [[A1, B1, C1]]
    • If you write into multiple rows/columns, array should be in this form: [[A1, B1, C1], [A2, B2, C2], [A3, B3, C3]]
  • I needed to separate the setValues in your case since you might have data in between columns B and D and it will overwrite those values. So I chose to do it by column which is still an improvement over multiple setValue
Related