0

I'm working in Google apps script and seem to have screwed up one of my for loops. I'm sure that I am missing something trivial here, but I can't seem to spot it.

Code Snippet:

var ss = SpreadsheetApp.getActiveSpreadsheet();
var sheets = ss.getSheets();
var lastRow = sheets[3].getLastRow();
var zw = sheets[3].getRange(2, 1, lastRow - 1, 26).getValues();
for (var j = 0; j < zw.length; ++j) {
    if (zw[j][9] === 'Yes') {
        var masterEmail = [];
        var firstLetterLastName = [];
        var first2Letter = [];
        var masterEmail.push(zw[j][22]);
        var firstLetterLastName.push(zw[j][1].charAt(0).toLowerCase());
        var first2Letter.push(zw[j][1].charAt(0).toLowerCase() + zw[j][1].charAt(1).toLowerCase());
        //The rest of the function follows...
    }
}

What's Not Working:

The for loop doesn't increment. When running the code in a debugger, var j stays at a value of 0.0, and the rest of the function only runs based of off the values in the 0 position of zw.

What I need it to do (AKA - How I thought I had written it:)

The ZW variable is holding a 2 dimensional array of cell values from a Google sheet. I'm looping through that, checking the 9th value of each array entry for a string of "Yes" and then running the rest of the function (for each column with a "Yes") if the condition is true.

I thought I had this working before, but recently had to restructure and optimize some things. Now I'm starting to think I may need to rethink things and use a different loop method. Can anyone educate me?

Edit: Here's a bit more context as requested:

function menuItem1() {
  var ui = SpreadsheetApp.getUi();
  var response = ui.alert('Are you sure you want to send emails?', ui.ButtonSet.YES_NO);
  var ss = SpreadsheetApp.getActiveSpreadsheet();
  var sheets = ss.getSheets();
  var lastRow = sheets[3].getLastRow();
  var zw = sheets[3].getRange(2, 1, lastRow - 1, 26).getValues();
    if (response === ui.Button.YES) {
      for (var j = 0; j < zw.length; j++) {
        if (zw[j][9] === 'Yes') {
          var firstLetterLastName = [];
          firstLetterLastName.push(zw[j][1].charAt(0).toLowerCase());
          //Other Stuff....
        }
      }
    }
}

I have a menu item attached to a simple onOpen, that calls menuItem1(). Calling the function prompts the user with a warning that they are about to send emails, then goes about getting data to assign email addresses based on the contents of the sheets. firstLetterLastName is an example.

I'm still not getting the loop to function, is it because I have it between two if statements? (Here is a link to the sheet)

5
  • could you show how the spreadsheet data looks like? Because there does not seem to be anything wrong with the for loop you've written. Jan 27, 2017 at 3:38
  • I added an edit, does it provide the data you were looking for?
    – Jensen010
    Jan 27, 2017 at 9:27
  • I made a copy of your spreadsheet, stripped the code out of your for loop and ifclauses and had no problem running your logic. If you aren't getting the expected results, I would focus on the code inside your for loop. If you would provide more information on what errors, if any, you're getting that would help us solve your problem.
    – Silinus
    Jan 27, 2017 at 14:53
  • The error I'm getting is that [ j ] is staying at 0 instead of incrementing through the data I need it to - zw[j][9]. That corresponds to column...U? in the spreadsheet, which asks the client if they would like to send an email. var zw has all of the expected data (an array of arrays with all cell contents of each row, for each internal array item) I am trying to loop through that, determine whether there is a yes in column 9 of each row, and then run the function based on that. But, since [j] stays at 0, it will only look at one of those rows. Not an "error" per se, I've just done it wrong :)
    – Jensen010
    Jan 27, 2017 at 20:01
  • 1
    I copied your file and started looking at your code. See my edits to the code in my copied file.
    – Karl_S
    Jan 27, 2017 at 20:37

1 Answer 1

7

Indeed it is quite trivial. You have mixed up your increment. You wrote

for (var j = 0; j < zw.length; ++j)

which means that you do 1 + i (and we know that at the start i = 0 which means your value will always be 1) instead of using the usual

for (var j = 0; j < zw.length; j++)

which would mean that you do i + 1 and update i, so you will get the expected 0 + 1 1 + 1 etc

EDIT:

First, I recommend instead of something like

if (responseMir === ui.Button.YES) {
  // Your For loop

doing

if (responseMir !== ui.Button.YES) {
  return
}

and in a similar fashion in the for loop

if (zw[j][9] !== 'Yes') {
  break
}

It mostly helps increase readability by not including large blocks of code under a single if, when all you want to do is to stop execution.

Your for loop gets broken because of the mistake here:

teacherEmailMir.push(selValsMir[j][7]);

So your loop will go over once. However on the next itteration, you try to push selValsMir[1][7] which does not exist. Note that each itteration you have var selValsMir = []; inside the loop, which means that for every j selValsMir will always be an empty array. So with the following line

selValsMir.push([zw[j][0], zw[j][1], zw[j][2], zw[j][3], zw[j][4], zw[j][5], zw[j][7], zw[j][22], zw[j][23], zw[j][24]]);

your array will always have selValsMir.lenght = 1 and selValsMir[0].length = 10. So obviously trying to access anything from selValsMir[1] will throw you an error and stop the script right there.

I also recommend looking over the if statements that look at the first and first 2 letters of the name as I believe you can accomplish the same with less code. Always try to streamline. Consider using switch() where you end up using a lot of else if

3
  • You are right, thank you :) I'm sure that was part of the problem, but I think I have another issue with it as well. I added an edit above and linked my sheet if you would like to take a look
    – Jensen010
    Jan 27, 2017 at 9:32
  • 1
    @Jensen010 I updated my answer. Seems like I have been working a lot with your particular script so far :D Anyway, try to use more whitespace. Leave a couple of blank lines to break up the code where the logic differs. For example inside the for loop, where you have all your var in one block, put an empty line before that block and after it. The readability will immediately increase and the code does not execute any different. Always use whitespaces
    – Vytautas
    Jan 28, 2017 at 11:05
  • Thank you! This makes a lot of sense. I had it capturing all of the data i needed before switching to the menu to act as the trigger, but some of the things I changed in that process must have messed it up. This will go a long way in figuring that out, and in adapting the whole thing to a more generalized web app (Karl_S comment above) Thank you again for all the helpful answers! :)
    – Jensen010
    Jan 30, 2017 at 16:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.