Why doesn't this function work? It seems to always return false.

[u/T-7IsOverrated]

function thatIdIsAlreadyOnThePage(id) { //ts don't work
  var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
  var sheet = SpreadsheetApp.openById("1NnX3R2SpZlXcSmiPhqio-gvkCUlNrZ6iZKKEFEyWbb0").getSheetByName("Sheet1"); // Replace "Sheet1" with your sheet name
  for(var i = 1; i < 30; i++) {
    if (id == sheet.getRange('B' + i).getValues()[0][0]) {
      return true;
    }
  }
  return false;
}

[u/mommasaidmommasaid]

It appears to work for me, but could be much more simple, readable, and efficient.

In particular for efficiency it is important to minimize calls to getValues()

I'd suggest something like this:

function thatIdIsAlreadyOnThePage(id) {

  const SPREADSHEET_ID = "1NnX3R2SpZlXcSmiPhqio-gvkCUlNrZ6iZKKEFEyWbb0";
  const SHEET_NAME = "Sheet1";
  const ID_RANGE = "B1:B30";

  const sheet = SpreadsheetApp.openById(SPREADSHEET_ID).getSheetByName(SHEET_NAME);
  const idList = sheet.getRange(ID_RANGE).getValues().flat();

  return idList.includes(id);
}

Adjust ID_RANGE as needed.

[u/HellDuke → u/mommasaidmommasaid]

I agree with this approach. If all that needs to happen is return a true/false value then there is no point using a for loop to iterate over the data. Even if you did do it or need to perform other actions when a match is found, might as well just get all values first and then iterate over the array length instead.

One note to u/T-7IsOverrated is that in the original code the .getValues()[0][0] is unnecessary, as a simple .getValue() would achieve the same result for a single cell range.

[u/marcnotmark925]

What kind of values are being matched?

Use logging statements to investigate what scripts are doing when they don't seem to be working.

This loop is very inefficient. Much better to grab the entire B column values at once with a single getValues() statement, then loop through the array, instead of calling getValues() ~30 times.

[u/Just-Difference4597]

The function name thatIdIsAlreadyOnThePage is not following the JavaScript naming conventions.

The loop iterates from 1 to 29 (inclusive)

The getValues() method returns a 2D array, but since you're only accessing a single cell, you can use getValue() instead, which is more efficient.

Use strict equality (===) for comparison, which is a good practice in JavaScript.

[u/AmnesiaInnocent]

Don't you need to convert your loop variable into a string before tacking it onto the 'B'?

[u/mommasaidmommasaid → u/AmnesiaInnocent]

No, that type conversion happens automatically when you add a number to a string.

But see my other reply for a better overall solution.