3
\$\begingroup\$

It all works exactly as it should. It finds data from today, finds unique emails and puts them in an array. I then check the data again from today, against the emails to total up different values. Then output those values to a sheet.

I'm sure there might be better methods to do what I wish. Such as only going through the data once, not using 3 arrays, etc. But it does all seem to work.

function todayByPerson() {
    var emailColumn =1;
    var dateColumn = 2;
    var findColumn = 4;
    var comFind = "Yes - Complete (going ahead)";  
    var ansFind = "No Answer";
    var intFind = "No - Not interested in taking part";
    var cbFind = "Callback";
    var tolFind = "Temp Offline (If TOL do not continue)";
    var target = "Today";

    var ss = SpreadsheetApp.getActiveSpreadsheet();
    var targetSheet = ss.getSheetByName(target);
    var sheet = ss.getSheetByName('Form Responses');
    var values = sheet.getDataRange().getValues();

    var today = new Date();
    var thisMorning = today;
    thisMorning.setHours(0);
    thisMorning.setMinutes(0);
    var archive = [];
    var names = [];
    var data = [];
    var completed = 0;
    var noAnswer = 0;
    var callBack = 0;
    var notInterested = 0;
    var tempOffline = 0;

    var totCom = 0;
    var totAns = 0;
    var totCal = 0;
    var totNot = 0;
    var totTol = 0;

    for (var counter =  1; counter < values.length; counter++) {
        var testDate = new Date(values[counter][dateColumn -1]);

        if (testDate < thisMorning) {
            continue;
        }

        if (archive.some(function (element, index, array) {
            return values[counter][emailColumn - 1] == element[emailColumn - 1];
        })) {
            continue;
        }

        archive.push(values[counter]);
    }

    var archiveLength = archive.length;

    for (i=0;i< archiveLength;i++) {
        names.push(archive[i][0]);
    }

    for (i=0; i<names.length;i++) {
        for (var counter =  1; counter < values.length; counter++) {
            var testDate = new Date(values[counter][dateColumn -1]);

            if (testDate < thisMorning) {
                continue;
            }

            if (values[counter][emailColumn -1] == names[i] &&
                values[counter][findColumn -1] == comFind) {

                completed++;
                totCom++;
            }

            if (values[counter][emailColumn -1] == names[i] &&
                values[counter][findColumn -1] == ansFind) {

                noAnswer++;
                totAns++;
            }

            if (values[counter][emailColumn -1] == names[i] &&
                values[counter][findColumn -1] == cbFind) {

                callBack++;
                totCal++;
            }

            if (values[counter][emailColumn -1] == names[i] &&
                values[counter][findColumn -1] == intFind) {

                notInterested++;
                totNot++;
            }

            if (values[counter][emailColumn -1] == names[i] &&
                values[counter][findColumn -1] == tolFind) {

                tempOffline++;
                totTol++;
            }
        }

        var attempts = noAnswer+callBack+completed+notInterested+tempOffline;
        var success = completed/attempts*100;
        data.push([names[i], attempts, completed, noAnswer, callBack, notInterested, tempOffline, success]);

        var completed = 0;
        var noAnswer = 0;
        var callBack = 0;
        var notInterested = 0;
        var tempOffline = 0;
    }

    var totAtt = totCom+totAns+totCal+totNot+totTol;
    var headers =[["Name", "Attempted", "Completed", "No Answer", "Callback", "Not Interested", "Temp Offline", "Success Rate"]];
    var footers =[["Totals", totAtt, totCom, totAns, totCal, totNot, totTol, totCom/totAtt*100]];  

    targetSheet.getRange(1,1,1,headers[0].length).setValues(headers);
    targetSheet.getRange(1,1,1,headers[0].length).setBackgroundColor("#43b9ff");
    targetSheet.getRange(1,1,1,headers[0].length).setFontColor("#a60303");
    targetSheet.getRange(1,1,1,headers[0].length).setFontWeight("bold");

    targetSheet.getRange(2,1).offset(0, 0, data.length, data[0].length).setValues(data);

    targetSheet.getRange(data.length+2, 1, 1, footers[0].length).setValues(footers);

    targetSheet.getRange(data.length+2, 1, 1, footers[0].length).setBackgroundColor("#43b9ff");
    targetSheet.getRange(data.length+2, 1, 1,footers[0].length).setFontColor("#a60303");
    targetSheet.getRange(data.length+2, 1, 1,footers[0].length).setFontWeight("bold");
}

EDIT

After I posted the above, I was working on another addition to this, something to change emails into nicer looking names for the report. All emails are in the following format.

firstname.lastname@email-domain.co.uk

so here is the additional snippet.

var cleanName = names[i].replace("@email-domain.co.uk","");
var cleanName = cleanName.replace("."," ");
var cleanName = cleanName.charAt(0).toUpperCase() + cleanName.slice(1);
var space = cleanName.search(" ");
var cleanName = cleanName.substr(0,space+1)+ cleanName.charAt(space+1).toUpperCase() + cleanName.slice(space+2);
\$\endgroup\$
  • \$\begingroup\$ A thought has struck me, I'll have to fill entire sheet white, and set font black before colouring ranges. Or sheet will end up all stripey as data changes. \$\endgroup\$ – Munkey Feb 5 '14 at 20:12
1
\$\begingroup\$

You keep repeating if (values[counter][emailColumn -1] == names[i] && and values[counter][findColumn -1] , how about doing that just once ?

You will end up with this more readable, faster code:

if (values[counter][emailColumn -1] == names[i]){

  var value = values[counter][findColumn -1];

  if (value == comFind) {
      completed++;
      totCom++;
  }

  if (value == ansFind) {
      noAnswer++;
      totAns++;
  }

  if (value == cbFind) {
      callBack++;
      totCal++;
  }

  if (value == intFind) {
      notInterested++;
      totNot++;
  }

  if (value == tolFind) {
      tempOffline++;
      totTol++;
  }
}

Furthermore, there are a number of problems with your variables ( redeclaring again or not declaring ), please use jshint.com to check those out. At the very least, you ought to group your vars on top, ideally with one comma-chained var statement.

Since headers and footers have the same color scheme, you might want to consider having a function to set the color for a range. This also makes it faster, since you are then not calculating the range over and over.

function setColors( range ){
  range.setBackgroundColor("#43b9ff");
  range.setFontColor("#a60303");
  range.setFontWeight("bold");
}

Also, your naming could be a bit more informative, I assume totTol stands for totalTotal which is an unfortunate name. You also have 0 lines of comment.

\$\endgroup\$
  • \$\begingroup\$ thank you for the reply and suggestions. I'm very new to Javascript, so always looking to learn. I'll try to implement the changes you have suggested and get back to you. I redeclared vars below again, such as var tempOffline = 0; so i could reset the counter per person. The var tolTot stands for tempOffline Total, I didn't want to use temp as a var or name. Many thanks for the feedback and help :) \$\endgroup\$ – Munkey Feb 6 '14 at 17:12
  • \$\begingroup\$ You can just call tempOffline = 0, you do not need var, for someone new to JS, your code is quite good. \$\endgroup\$ – konijn Feb 6 '14 at 17:17
  • \$\begingroup\$ Ahhh, awesome, rookie error, I don't need to redeclare vars. Just change value. Never seen jshint.com, just checked it, great link. Bookmarked! \$\endgroup\$ – Munkey Feb 6 '14 at 17:43
  • \$\begingroup\$ I managed to implement all but one of your suggestions and things are much better. Wish i could post revised code here, not enough space. One thing I didn't get 100% (only been learning Javascript since xmas) is "you ought to group your vars on top, ideally with one comma-chained var statement." you stated. Something like var ss, sheet, values, valuesLength, counter, archive = [];??? OK, so possibly/surely/100% stupid question, but why? If you have to go and define most of them anyways or give them a values? I'd be writing them twice, or no? \$\endgroup\$ – Munkey Feb 7 '14 at 2:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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