Why is my for loop running an extra iteration and not copying to the correct cells

Question

When I run my (messy) script it seems to run one extra time than required. i.e. below the last row it creates a pdf and marks a cell as processed.

The second issue is that the URLs don't seem to line up correctly with the name that it should be.

I appreciate the code is very messy but I'm happy to explain the reasoning for any part if it doesn't make sense.

Thanks in advance for any help!

The spreadsheet can be found here:

https://docs.google.com/spreadsheets/d/1nq5RRcwAKk9sFm6jVypFT_qJWcOGxFDZRv6ZpB-QClw/edit#gid=1194576382

and the code that's not working as expected is:

var ss = SpreadsheetApp.getActiveSpreadsheet()
var rawData = "rawData"
var practicePivot = "practicePivot"
var querySheet = "querySheet"
var pdfSheet = "pdfSheet"
var contactList = "contactList"

function createPDF(){

var sourceSheet = ss.getSheetByName("querySheet")
var pdfList = ss.getSheetByName("practicePivot")
var contactList = ss.getSheetByName("contactList")
var sourceRow = sourceSheet.getLastRow()
var sourceColumn = sourceSheet.getLastColumn()
var sourceStartRow = 4 //skips the headers and only pulls query data
var sourceStartColumn = 1
var sourceRange = sourceSheet.getRange(sourceStartRow, sourceStartColumn, sourceRow, sourceColumn)
var sourceValues = sourceRange.getValues()
var pdfLastRow = pdfList.getLastRow()
var storePracticeName = sourceSheet.getRange("A2").getValues()


var newSpreadsheet = SpreadsheetApp.create("Summary of Patients for" +storePracticeName)

sourceSheet.copyTo(newSpreadsheet, {contentsOnly: true})
newSpreadsheet.getSheetByName("sheet1").activate()
newSpreadsheet.deleteActiveSheet()


var pdfURLtemp = DriveApp.getFileById(newSpreadsheet.getId()).getAs("application/pdf")
var pdf = DriveApp.createFile(pdfURLtemp)
var pdfURL = pdf.getUrl()
Logger.log(pdfURL)
return pdfURL


}


function createQuery() 
{

//Duplication check
var pdfCreated = "pdfCreated";
var pdfEmailed = "pdfEmailed";

var sheet = ss.getSheetByName("practicePivot");
var startRow = 2; 
var lastRow = sheet.getLastRow();
var lastColumn = sheet.getLastColumn();

var dataRange = sheet.getRange(startRow, 1, lastRow, lastColumn)  ;
var data = dataRange.getValues();


for (var i = 0; i < data.length; ++i)
  {

    var row = data[i];
    var practiceName = row[0]
    var pdfCheck = row[2]

    var copySelection = sheet.getRange(startRow + i, 1)
    var copyData = copySelection.getValues()
    var copyLocation = ss.getSheetByName("querySheet")
    var copyCell = copyLocation.getRange("A2")

    if (pdfCheck != pdfCreated)
      {
      var pdfURL = createPDF()
      copyCell.copyTo(copyData)
      sheet.getRange(startRow + i, 4).setValue(pdfURL)
      sheet.getRange(startRow + i, 3).setValue(pdfCreated)
            SpreadsheetApp.flush()

      }

  }
}

Try using `i++` not `++i` [Stack Overflow answer](https://stackoverflow.com/a/6867939/2946873) — Alan Wells, Jun 29 '17 at 14:02
Hi Sandy, thanks for the suggestion. I've just tried i++ and it still created an extra pdf. The extra pdf is "blank" and is put in the place where test 1 pdf should be. "test 1" pdf is placed in the blank row. — Luke, Jun 29 '17 at 14:31

score 0 · Accepted Answer · answered Jun 29 '17 at 15:15

0

Your start row is 2:

var startRow = 2;

Your loop is looping until the count of the last row. So if there were 10 rows, and the data starts in row 2, then you need the loop to run 9 times. But your loop is running 10 times.

So you need to adjust the stop for the loop.

var L = data.length - 1;//The number of rows in the data - not the sheet
for (var i = 0; i < L; ++i)

answered Jun 29 '17 at 15:15

Alan Wells

30,746
15
104
152

I'm at home right now so can't check it, but that makes sense to me! Thanks for the help! Would that be the cause of the setValues not being in the right cells as well? – Luke Jun 29 '17 at 16:50
No, that is caused by `++i` The first time that the loop runs, `i` is the value of 1. So, the start row starts at 3. If this solves your problem you can click the big green check mark to mark the answer as correct. – Alan Wells Jun 29 '17 at 17:09
Hi, So I've put in that extra variable. And it doesn't do the extra loop however the PDF it creates does not have the data in it that lines up with the correct data. I've used i++ in it as well. I'm also a little confused as to why the data.length - 1 is needed. If i set the data range as 2 - 10 and getValues for that, wouldn't it only return 8 values therefore giving length as 8? Leading to i = 0 going up to i = 7? Or does the i < length mean it runs 0 - 8 instead of 0 - 7? Sorry teaching myself to code and not doing that well! – Luke Jun 30 '17 at 07:45
If the start row is 2, then row 2 is being included. So, 2 rows are not being "taken away." 10 - 2 = 8 But 2 rows are not being taken away. Only row 1 is not being included. Row 2 doesn't mean that 2 rows are being excluded. Only one row is being excluded. To make it even easier to prove, imagine if there were only 4 rows, and the start row was 2. Row 2, Row 3, Row 4 That's 3 rows. You must include row 2 in the count. This is one of those situations where the human mind plays tricks on you. We don't always perceive reality. – Alan Wells Jun 30 '17 at 13:00
Ah like the Bellhop only giving back 1 dollar to each man puzzle. I now get it I think. Thanks for the help! I think I'll go through the code again and work out why it isn't creating the correct pdf at the correct time. If it's not the ++i vs i++ then I suspect i've mucked up the code. Thanks again for the solution AND the explanations! – Luke Jun 30 '17 at 13:12
I think I worked out why the PDF wasn't copying correctly. I had: var pdfURL = createPDF() copySelection.copyTo(copyCell) Which I think meant the createPDF() was being called whilst there was no data in the cell. – Luke Jul 04 '17 at 07:41

Why is my for loop running an extra iteration and not copying to the correct cells

1 Answers1