Scripts, scripts, scripts!

@Drearystate @Wiz.Wazeer - hey guys, have either of you actually tried to execute the above script? As it is, it is severely broken, and there is also one fundamental implementation flaw.

The most serious issue is this:

The above loop starts at the top of the sheet and works its way down, deleting any rows that match the IF condition. The problem with this is that as soon as the first row is deleted, all the remaining index numbers are offset by 1, and so when it hits the next row to be deleted, it will delete the WRONG row.

To avoid this problem, you need to iterate in reverse order - that is, start from the bottom and work your way up.

Other issues:
SpreadsheetApp.setActiveSheet(getSheetByName('Sheet1'))

The above line will throw an error. I’m not quite sure what Joe was trying to do there, but he seems to be trying to chain methods from different classes, which won’t work.

I probably would have done something like:
var sheet = ssActive.getSheetByName('Sheet1');

And then the next line becomes:
var MyRange = sheet.getRange("A:A"); // selects column A
(but see below)

The next issue is in this line:
var ADayAgo = new Date(date.getTime()-(300*1000));

Which throws an error because date is not defined. That should be re-written as:
var ADayAgo = new Date().getTime()-(300*1000);

The next problem is this line:
var MyRange = sheet.getRange("A:A"); //selects column A

That line will return a RANGE object, but not the actual values in the range. The result of this is when you try to iterate through the data further down, there is no data to iterate through, and so the loop doesn’t actually do anything.

So that line above should have been:
var MyRange = sheet.getRange("A:A").getValues(); //selects column A

Other minor issues are around the choice of variable names, which are very misleading. But I guess that’ s just because Joe copy/pasted from another script and didn’t bother changing any of them. I always try and make my function/variable names as descriptive as possible. For my own sanity more than anything else - because I know I’m going to have to come back and debug 3 or 6 months later.
Also, because this is essentially a utility function, and is likely to be re-used, I would try and generalise it as much as possible.

Anyway… all that being said, here is how I would do it. (tested and verified as working)

function delete_rows_older_than(sheetname, mins, col_number) {
    // The function name makes it clear what the purpose is
    // Allowing it to accept parameters makes it flexible
    // sheetname: which sheet to operate on
    // mins: how long before data is considered stale, and can be deleted
    // col_number: which column contains the date/time stamp (A=1, B=2, C=3, etc)
    
    var ss = SpreadsheetApp.getActiveSpreadsheet();
    var sheet = ss.getSheetByName(sheetname);
    var ms = mins * 60 * 1000; // Convert minutes to milliseconds
    var compare_time = new Date().getTime() - ms;
    
    var last_row = sheet.getLastRow(); // Find our starting point
    if (last_row < 2) { return ;}  // Just in case the sheet is empty

    for (var row = last_row; row > 1; row--) {  // Start from the bottom and work our way up
      var row_time = sheet.getRange(row, col_number).getValue().getTime();
      if (row_time < compare_time) {
        console.log("Deleting Row %s", row);
        sheet.deleteRow(row);
      }
    }
  }

And the above would be called with:

function clear_data() {
    delete_rows_older_than('Sheet1', 5, 1);
  }
7 Likes