Nsryjdtyk

So right now I've got this function that's called in several places in some of my Google Spreadsheets. Right now it accepts a Range object, a string to search for, and an optional boolean to determine whether to make the search case-sensitive or not, and defaults to true.

function getCellsWithValueInRange(SearchRange, search_string, case_sensitive) {

    var case_sensitive = (case_sensitive !== undefined && case_sensitive === false) ? false : true;

    var result = ;

    var RangeValues = { values: SearchRange.getValues(),

                         width: SearchRange.getWidth(),

                        height: SearchRange.getHeight(),

                         start: { column: SearchRange.getColumn(),

                                     row: SearchRange.getRow() }};

    for (var x = 0; x < RangeValues.width; x++) {

        for (var y = 0; y < RangeValues.height; y++) {

            if (!case_sensitive) {

                // Lower case the search term and potential result, but ONLY if either terms have the toLowerCase method (like date objects).

                search_string = search_string.toLowerCase ? search_string.toLowerCase() : search_string;

                RangeValues.values[y][x] = RangeValues.values[y][x].toLowerCase ? RangeValues.values[y][x].toLowerCase() : RangeValues.values[y][x]; 

            }

        if (search_string === RangeValues.values[y][x]) {

            result.push(SearchRange.getCell(y + RangeValues.start.row, x + RangeValues.start.column));

        }

      }

    }

    return result;

}

Right now, this code works, but I'm not terribly happy with for several reasons:

• It uses multiple nested loops, which feel and look horrible (debugging this was a pain).


• I have doubts on its performance. Right now I've attempted to follow the best practices by batching retrieval (the first draft of this code just directly referenced the specific Ranges via getCell in the nested loops, but performance was so bad that any range that was over 600 rows just simply gave up, hence the use of getValues() instead. I haven't looked at the performance of this version, but at least it will accept large ranges (approximately 12 columns × 1300+ rows) without failing because it exceeded the maximum execution time.


• I have no idea if there are any search values that would cause this function to fail. I mean, it already throws an error if the Cell Range is an object (i.e. when it's a date). Where else would it fail? Can I protect myself from other instances?

In any case, are there better, more maintanable and performant ways to code this? I've kind of accepted that it won't exactly be the fastest code (it literally has to go through any kind of range, no matter it's state), but any kind of improvement will do.

I'm kind of stymied, mostly because after searching high and low, I've discovered that it's kind of hard to get indexes for nested Arrays in JavaScript (and, by extension, Google Apps Script), so my initial attempts to make this more functional by using map or filter (I was toying with using Underscore for Google Apps Script) just hit a brick wall.

I didn't run unit tests for this on different search inputs, but it should give an idea of how you can bundle index information into the results for consumption with array class methods. Because you are working in a fairly robust environment (Google Sheets), you can be confident that the search array you are working with has a guaranteed nesting level (which limits a lot of possible issues).

The general idea is to replace the raw value with an annotated object, after which you can remove the row & column structure and work only with the value. (It's possible you could just use the reduce | forEach and push Array methods to go from the values array to the flattened array of annotated values.)

function annotateAndCase_(input2DArr, lower) {

  // Map the 0-base row and column indices onto the value.

  return input2DArr.map(function (row, rowIndex) {

    return row.map(function (val, colIndex) {

      var v = (lower && typeof val.toLowerCase === 'function') ? val.toLowerCase() : val;

      return {r: rowIndex, c: colIndex, v: v};

    });

  });

}



function getCellsWithValueInRange(SearchRange, search_string, case_sensitive) {

  // Pass exactly `false` to disable case sensitivity. Other falsy (0, "", null, undefined) -> `true`

  case_sensitive = case_sensitive !== false;

  if (case_sensitive) {

    search_string = search_string.toLowerCase();

  }

  const startRow = SearchRange.getRow(),

        startCol = SearchRange.getColumn();



  const indexedValues = annotateAndCase_(SearchRange.getValues(), !case_sensitive);



  // 2D array with max nesting level = 1 -> https://stackoverflow.com/a/10865042

  const flattened = Array.prototype.concat.apply(, indexedValues);



  const results = flattened.filter(function (cell) {

    return cell.v === search_string;

  }).map(function (match) {

    // Return a string R1C1 notation of each matched value.

    return ["R", match.r + startRow, "C", match.c + startCol].join("");

  });



  // Use a RangeList to optimize obtaining a large number of possibly-disjoint ranges.

  return results.length ? SearchRange.getSheet().getRangeList(results).getRanges() : ;

}

As mentioned in comments by @Dimu, you should reconsider why you need the individual cell references (perhaps just the collection as a RangeList is sufficient, e.g. for treating them in the same manner).

It's a similarly good idea to separate the functionality of annotating from this specific function that searches for values, since that allows reusing code, e.g. annotated a range object, use various filter predicates, various transformative maps, various final consumers, and so on. Probably you should further separate the casing from the annotation step - only certain filter predicates for the annotated array would care.