Sjsyrek edits

[sjsyrek]

To help replace some of the magic numbers, I started by replacing API calls to sheet.getLastRow() with a closure that can compute an offset with a parameter.

[andydavies]

I'm not sure this is any clearer…

sheet.getLastRow() - 1 is calculating the number of rows in the range minus the title row so perhaps we should do something like

titleRow = 1; sheet.getLastRow() - titleRow…

WDYT?

[sjsyrek]

Right! Maybe a good compromise would be to make a function that just does that calculation and bind it to a variable so you can at least put a name to it:

var lastRow = sheet.getLastRow() - l;

Or in a more functional style:

var lastRow = function() { return sheet.getLastRow() - 1 }

My understanding is that we'd like to minimize calls out to the Apps APIs, but that may be impossible if the sheet is modified once it's loaded (and this one is virtually instantaneous, server slowdowns notwithstanding). The variable may only be evaluated when the sheet is loaded (I'm not sure) and then it wouldn't return the correct value. The closure version will call the function whenever it's required, at least, at the expense of having ugly parens after the identifier, but at least you'll know that something special is happening and won't have to insert a sheet.getLastRow() - l wherever you need that. You could certainly pair that up with a variable holding the number of title rows you want to exclude—good idea! So:

var titleRows = 1 var lastRow = function() { return sheet.getLastRow() - titleRows; }

A better name for lastRow might be rangeHeight or something, if it's going to confuse people, but if I've overthought this too much already, that's perhaps a bridge too far.

[sjsyrek]

It's been a year, so I assume this isn't being merged. Just doing some PR housekeeping :)