skip to Main Content

I’ll be giving a bit of context:

Using App script, I’m working on a macro which copies data from a sheet to another. To avoid copying mistakes cascading, I added code to check whether or not the cells are empty. There are 7 cells to check and the first or 5th one may be empty. (they either are both filled, or one of the 2 is blank, if the 2 are blank it’s invalid), to simplify I made it so that the ranges are always continuous and on a single line, so only the columns change.

To achieve my goal I did a simple counting loop:

var rangeArray = rangeToArray(range);
var emptyCells = 0;
for (var o = 0; o < rangeArray.length; o++) {
  if (sheet.getRange(rangeArray[o]).isBlank()) { emptyCells ++; }
}

and only copy if the value of emptyCells is < 2 (it is presumming the cells they leave empty are the correct ones but I can’t reasonably account for all human errors).

(Please ignore the var, I tried using let, but for obscure reasons App Script doesn’t support it)

range is the range in the A1 format such as "B2:H2",

emptyCells is the counter of empty cells,

sheet.getRange(rangeArray[o]).isBlank()) returns true when the cell is blank, false otherwise.

rangeToArray(range) however is where my question lies in, it’s a custom function I made which turns range into an array of cells, such as "B2:H2" → ["B2", "C2", "D2", "E2", "F2", "G2", "H2"]

Because it has to account for ranges anywhere between A1 and AZ999…, it has to work for both 1 letter and 2 letters column indexes on both the 1st and 2nd cell indexes as well as for any finite row indexes.

The code displayed below works, however, It’s having about 1.8 +/- 0.2 seconds execution time which is slow, even for a Google Sheet macro. (simplier macros on that sheet run between 0.3 and 1.4 seconds)
I’m not a JS dev, I started learning it a little over a week ago specificaly for that project; so it’s the best I could do after spending a few days on the matter but I have no idea how good or bad that actualy is. If anyone has suggestions for improvements or see obvious optimisations, please go ahead I’m all hears.

P.S.: I already tried adding an if statement to break out of the for loop in as soon as both the 2nd and 4th letters are found but it actualy increases execution time by 0.2 seconds on average.

My range to array of cells converter is as follows:

function rangeToArray(range) {
  var posColon = range.indexOf(":"); // position of the colon
  var posSecondLetter = 0; // position of the 2nd letter if there is one, the 1st one otherwise
  var posFourthLetter = posColon+1; // position of the 4th letter if there is one, the 3rd one otherwise
  var posTemp1 = -1;
  var posTemp2 = -1;
  var rangeArray = [];
  // Loops through all 26 letters to find if it matches with a potential 2nd or 4th letter
  for (var k = 0; k < 26; k++) {
    posTemp1 = range.indexOf(letterArray[k], 1);
    posTemp2 = range.indexOf(letterArray[k], posColon+2);
    if (posTemp1 != -1 && posTemp1 < posColon) {
      // it found what the 2nd letter is if there is one
      posSecondLetter = posTemp1;
    }
    if (posTemp2 != -1) {
      // it found what the 4th letter is if there is one
      posFourthLetter = posTemp2;
    }
  }
  // isolate the according column indicators of the 1st and 2nd cell as well as their row numbers
  var firstCellColumnIndex = numberOfLetter(range.slice(0, posSecondLetter+1));
  var secondCellColumnIndex = numberOfLetter(range.slice(posColon+1, posFourthLetter+1));
  var firstCellRowIndex = range.slice(posSecondLetter+1, posColon-posSecondLetter);
  var secondCellRowIndex = range.slice(posFourthLetter+1, range.length);
  //generating the array of cell inbetween and including them
  for (var row = firstCellRowIndex; row <= secondCellRowIndex; l++) {
    for (var col = firstCellColumnIndex; col  <= secondCellColumnIndex; m++) {
      rangeArray.push(letter(col)+row.toString());
    }
  }
  return rangeArray;
}

letterArray is just an array of all capital letters in alphabetical order, I made it for 6 other functions to convert from numerical index to alphabetical since unlike Ada, JS can’t count in alphabetical indexes, so it came in handy there.

numberOfLetter() is a custom function which takes a capital letter character or pair of characters and returns their corresponding numerical index, the reciprocal of letter(). ("A" → 1, "AZ" → 52)

letter() is a custom function which takes a numerical index and returns their corresponding alphabetical index. (1 → "A", 52 → "AZ")

I doubt there’s much to improve in letter() or numberOfLetter(), unless there’s a trick I don’t know about for loops but here they are if you wish.

function letter(number) {
  if (number <= 26) { //indexes from A to Z
    return listeLettres[number-1];
  } else if ((26 < number) && (number <= 52)) { //indexes from AA to AZ
    return "A"+listeLettres[number-27];
  }
}

function numberOfLetter(letter) {
  for (var i = 0; i < 26; i++) { //indexes from A to Z
    if (letter == listeLettres[i]) {
      return i+1;
    }
  }
  for (var j = 0; j < 26; j++) { //indexes from AA to AZ
    if (letter == "A"+listeLettres[j]) {
      return 26+j+1;
    }
  }
}

(I’m aware the multiple returns are bad practice but considering I’m forced to have all variables as global thanks to App Script I’d rather limit unecessery intermediary variables where I can + it also seems to shorten execution time and the lower I can get that, the happier I am xD)

EDIT: 21/03

People seem confused by the title and what/why exactly I’m trying to do things. So, a more "accurate title" would be "how can I optimise X program to reduce execution time" where that program is this:

function transfertUTI(range) {
//Part 1: Checking if no more than 3 cells are empty so that it doesn't execute if it's not filled properly
  var rangeArray = rangeToArray(range); //Converts range to array of cells
  var emptyCells = 0;
  for (var q = 0; q < rangeArray.length; q++) {
    if (sheetUTI.getRange(rangeArray[q]).isBlank()) {
      emptyCells++; // counts how many cells in the range are empty
    }
  }
  if (emptyCells <= 2) {
//Part 2: Actualy do the program
    gatherIntoListIndexes(); //This generates listIndexes which is an array with the coordonates of the topleftmost cell of all tables in the document which is used for nearly everything
    var AvailableLine = searchBlank(sheetUTI, listIndexes[4], listIndexes[5], 2); //this looks for the topmost empty line in the table it's transfering to, and returns the corresponding range at the specified range length, in that case 3 columns long
    sheetUTI.getRange(range).copyTo(sheetUTI.getRange(AvailableLine), {contentsOnly:true}); //the copy function of app script which copies from the first table to the second table
    sheetUTI.getRange(rangeShift(AvailableLine, 3, 1)).uncheck(); //rangeShift takes in a range, shifts it by as many columns as specified (here 3), and reduces its length as specified (here 1), which corresponds to the 2 checkboxes to uncheck next to where the data was just copied
    sheetUTI.getRange(range).clearContent(); //Empties the data on the initial table

  }
}

However, the reason for my question being the way it is and the title being what it is, is that from my PoV the only serious optimisation possible in that code is in the 1st part (rangeToArray + checking for empty cells loop) and the way to deal around the constraints of the .CopyTo() function that is how I have to first copy and then uncheck the checkboxes.
And both of those are 2 key functions that are rangeToArray() and rangeShift() which use the same core algorithm to work.

They divide the range into 4 parts, the first letter cluster, first number cluster, second letter cluster and second number cluster; which corresponds respectively to the 1st cell’s column index, 1st cell’s row index, 2nd cell’s column index and 2nd cell’s row index, which I can manipulate independantly to get what I want.

In rangeToArray() it’s so that I can generate the according array of cells, and in rangeShift() it’s so that I can manipulate the indexes individualy and regenerate a shifted range from an input range regardless of its details.

So a way to optimise one may optimise both and/or be more optimal than just using both as is. And that’s why I considered it more appropriate to call it that directly rather than "Can X be optimised ?", given I can’t realy phrase X in a way that includes all the important details and isn’t a paragraph long.

Anyway, if you want explanations about that function:
It’s a macro triggered by a button, or a checkbox to work on mobile via the onEdit() function. Its purpose is to transfer the data of a container from the table of those that stay on the terminal to the one of those who will be loaded in a freighter. (for now both tables are on the same sheet (sheetUTI) however they may be splitted into 2 sheets)
Since there are as many buttons as there are lines (each line is a container), the function needs as input the range of the line in question to transfer the data of the right line.
There are 3 columns of data, so a container’s data may be the range "J6:L6" for exemple.

Then, in the table where it’s transfered to, there’s 2 additional columns added at the end which indicate the loading progress. Those need to be unchecked (reset) when data is transfered.

(Here’s rangeShift so that you can see how the only difference is the return’s content)

function rangeShift(range, shift, length) {
  var posColon = range.indexOf(":");
  var posSecondLetter = 0;
  var posFourthLetter = posColon+1;
  var posTemp1 = -1;
  var posTemp2 = -1;
  for (var k = 0; k < 26; k++) {
    posTemp1 = range.indexOf(listeLettres[k], 1);
    postemp2 = range.indexOf(listeLettres[k], posdp+2);
    if (posTemp1 != -1 && posTemp1 < posColon) {
      posSecondLetter = posTemp1;
    }
    if (posTemp2 != -1) {
      posFourthLetter = posTemp2;
    }
  }
//1st cell's column index
  return letter(numberOfLetter(range.slice(0, posSecondLetter+1))+shift)+
//1st cell's row index
range.slice(posSecondLetter+1, posColon-posSecondLetter)+
":"+
//2nd cell's column index
letter(numberOfLetter(range.slice(posColon+1, posFourthLetter+1))+shift-length)+
//2nd cell's row index
range.slice(posFourthLetter+1, range.length);
}

2

Answers


  1. function loadRange(name="Sheet1",row=5,col=5,rg="B2:H2") {
      const ss = SpreadsheetApp.getActive();
      const ssh = ss.getSheetByName("Sheet0");
      const vs = ssh.getRange(rg).getValues();
      const dsh = ss.getSheetByName(name);
      dsh.getRange(row,col,vs.length,vs[0].length).setValues(vs);
    }
    
    Login or Signup to reply.
  2. I have to agree with TheMaster. Seems like a lot of work for nothing.

    I creat a sheet as shown.

    enter image description here

    Then a test script. Notice I get the first element of the 2D array from getValues()[0]. Using => arrow function

    function test() {
      try {
        let spread = SpreadsheetApp.getActiveSpreadsheet();
        let sheet = spread.getSheetByName("Sheet1");
        let range ="A1:J1";
        let values = sheet.getRange(range).getValues()[0];
        let emptyCells = 0;
        values.forEach( cell => {
            if( cell === "" ) emptyCells++;
          }
        );
        console.log(emptyCells);
      }
      catch(err) {
        console.log(err);
      }
    }
    

    Using traditiona function function(cell) {

    function test() {
      try {
        let spread = SpreadsheetApp.getActiveSpreadsheet();
        let sheet = spread.getSheetByName("Sheet1");
        let range ="A1:J1";
        let values = sheet.getRange(range).getValues()[0];
        let emptyCells = 0;
        values.forEach( function (cell) {
            if( cell === "" ) emptyCells++;
          }
        );
        console.log(emptyCells);
      }
      catch(err) {
        console.log(err);
      }
    }
    

    And the execution log shows 2 empty cells.

    6:19:36 AM  Notice  Execution started
    6:19:36 AM  Info    2
    6:19:36 AM  Notice  Execution completed
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search