I'd like feedback on my first working script. Where can I improve in the future?

Question

This code is for a spreadsheet that tracks weekly picks for NFL games. Four friends and I pick winners for every NFL game each week. Suppose there are 18 games being played this week; for each game, we all pick a winner, and also a level of confidence in that winner. We rank each game 1 to 18 based on our confidence-- so if a really good team plays a really bad team, we'll pick the good team for 18; if two teams are evenly matched, we'll pick a team for 1. (Each rank may only be used once each week per player.) If our team wins, we get the number of points we put on the game. If our team loses, we get no points.

The spreadsheet displays all of the games each week, and we enter our rankings under our column in the row of the corresponding team (see spreadsheet), and on the same side as the team we want to win. Then when a game is over, we just have to enter "w" or "l" in the appropriate column, and all player's scores are automatically calculated. The code also attempts to inform us as soon as a player is eliminated from contention for top score of the week.

Here is the currently implemented method of setting an elimination status: 1) Start by taking the current scores into account. 2) Consider each player vs each other player (aka opponent) one at a time 3) If there is ever a time when a player cannot beat one or more of his opponents, then he is eliminated.

To check whether he can beat an opponent is tricky, though-- because the best case scenario for a player is not necessarily getting all of his picks right. Suppose they both pick the same team, but the opponent ranks that game higher-- it's actually better for the player (at least against this opponent) for the team he picked to lose.

So instead, I calculate a best possible scenario score (again, for each player against each individual opponent). It starts with each player's current score. Then for all unplayed games, if the player and the opponent picked opposite sides, then the player's pick is added to his score (and the opponent gets no points in this scenario.) If they picked the same team, then we have to check who ranked the game higher; if the player ranked the game higher, then we add his points to his score, AND we add the points the opponent put on the game to the opponent's score. If the opponent picked higher, then nothing happens, since the best outcome is that that team loses and no one gets any points.

At the end of all of that, if the best possible player score is smaller than the opponent's score in this scenario, then that player is eliminated.

It does a few other things as well-- there's a comment section of the spreadsheet that detects the user who entered the comment, colors it appropriately, then shifts all the comments up one cell.

Following is the code, and here is a link to the spreadsheet into which we enter the information.

function onEdit(event){
  var ss = SpreadsheetApp.getActiveSpreadsheet();
    var sheet = ss.getSheets()[0];
    var sh = event.source.getActiveSheet();
    var r = event.source.getActiveRange();
    var user = Session.getUser();
    var lastCellModified = r.getA1Notation();
    //only update the elimiantions/must win/lose scenarios if the w/l column is updated:
    if (lastCellModified == "A10" || lastCellModified == "A11" || lastCellModified == "A12" || lastCellModified == "A13" 
  || lastCellModified == "A14" || lastCellModified == "A15" || lastCellModified == "A16" || lastCellModified == "A17" 
  || lastCellModified == "A18" || lastCellModified == "A19" || lastCellModified == "A20" || lastCellModified == "A21" 
  || lastCellModified == "A22" || lastCellModified == "A23" || lastCellModified == "A24" || lastCellModified == "A25"){
      ss.getRange("D2:D7").clearContent();
      //clear "elimination" status, so it doesn't have to be done manually if a wrong game outcome is entered.
      var sheetArray = [];
      sheetArray = ss.getRange("A1:N25").getValues()
        //A1: [0][0]
        //A2: [1][0]
        //B1: [0][1]
        // IN OTHER WORDS: [Y][X] AND REMEMBER Y IS THE NUMBER, X IS THE LETTER
        //BECUASE I AM DUMB HERE IS A HANDY ALPHANUMERIC CONVERSION CHART:
        //A B C D E F G H I J K  L  M  N  O  P  Q  R  S  T  U  V  W  X  Y  Z
        //0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25

        //ALSO BECAUSE IT IS FUNNY TO ME HERE IS A HANDY NUMERONUMERIC CONVERSION CHART:
        //COLUMN DESIRED       : 1  2  3  4 5 6 7 8 9 10 11  12  13  14  15  16  17  18  19  20  21  22  23  24  25  26   27
        //ARRAY NUMBER TO ENTER:[0][1][2][3]4 5 6 7 8 9  10  11  12  13  14  15  16  17  18  19  20 [21][22][23][24][25][26]

        //player values: Steve=1, Chris=2, Tyler=3, Billy=4, Drew=5  
        //go through each player one at a time
        for (var player=1; player<6; player++){
          if(player==1){var pScore=sheetArray[1][1];var leftPCol=2; var rightPCol=11; var elimRow=1}
          if(player==2){var pScore=sheetArray[2][1];var leftPCol=3; var rightPCol=10; var elimRow=2}
          if(player==3){var pScore=sheetArray[3][1];var leftPCol=4; var rightPCol=9; var elimRow=3}
          if(player==4){var pScore=sheetArray[4][1];var leftPCol=5; var rightPCol=8; var elimRow=4}
          if(player==5){var pScore=sheetArray[5][1];var leftPCol=1; var rightPCol=12; var elimRow=5}
        //Now we take select opponents one at a time for the current player
            for(var opponent=1; opponent<6; opponent++){
              if(player==opponent){break}
                if(opponent==1){var oScore=sheetArray[1][1]; var leftOCol=2; var rightOCol=11}
                if(opponent==2){var oScore=sheetArray[2][1]; var leftOCol=3; var rightOCol=10}
                if(opponent==3){var oScore=sheetArray[3][1]; var leftOCol=4; var rightOCol=9}
                if(opponent==4){var oScore=sheetArray[4][1]; var leftOCol=5; var rightOCol=8}
                if(opponent==5){var oScore=sheetArray[5][1]; var leftOCol=1; var rightOCol=12}


                //Set best possible scenario score initially to the player's current score, and opponent's to opponent's current score
                var bPScore=pScore;
                //toastMessage = toastMessage + "bPScore for player "+player+" is "+bPScore+", ";
                var bOScore=oScore;
                //toastMessage = toastMessage + "bOScore for opponent "+opponent+" is "+bOScore+", "

                  for(var row=9; row<24; row++){ //Iterate through game rows.
                    if(sheetArray[row][0]==""){ //But only if the game has not been played yet

                      //determine which team player and opponent picked and if they picked the same team. If their left column ISN'T empty, they picked the team on the left; 
                      //if it IS empty, they picked the team on the right.
                      if(sheetArray[row][leftPCol]!=""){var pPick="left"} else{var pPick="right"}
                      if(sheetArray[row][leftOCol]!=""){var oPick="left"} else{var oPick="right"}
                      if(pPick==oPick){var sameTeam=true} else {var sameTeam=false} 

                      //get values of each pick
                      if(pPick=="left"){var pV=sheetArray[row][leftPCol]}
                      if(pPick=="right"){var pV=sheetArray[row][rightPCol]}
                      if(oPick=="left"){var oV=sheetArray[row][leftOCol]}
                      if(oPick=="right"){var oV=sheetArray[row][rightOCol]}
                      //if both players picked the same team, add the picks to the scenario score if player picked higher than opponent
                      if(sameTeam==true){
                        if(pV>oV){bPScore=bPScore+pV;bOScore=bOScore+oV}//if player picked for more points, add each players' picks to their score
                      }//if opponent picked for more points, don't do anything.

                      //if both players picked opposite teams, add player's pick value to his total score
                      if(sameTeam==false){bPScore=bPScore+pV} //don't change opponent's bp scenario score
                    }//end of cycling through opponent
                  }
                //After checking each unplayed game by the current player against a given opponent and assigning points based on a best case scenario, check if player is eliminated
                if (bPScore<bOScore){sheetArray[elimRow][3]="ELIMINATED"};             
            }
        }
      // sheet.getRange("O26").setValue(toastMessage);
      //var elimArray = [[sheetArray[1][3]],[sheetArray[2][3]],[sheetArray[3][3]],[sheetArray[4][3]],[sheetArray[5][3]]];
      //sheet.getRange("A26").setValue(sheetArray); //for debugging
      sheet.getRange("D2").setValue(sheetArray[1][3]);
      sheet.getRange("D3").setValue(sheetArray[2][3]);
      sheet.getRange("D4").setValue(sheetArray[3][3]);
      sheet.getRange("D5").setValue(sheetArray[4][3]);
      sheet.getRange("D6").setValue(sheetArray[5][3]);
      //end of ELIMINATIONS part of script
    }

  //This is for the moving comment box. It formats a cell with the colors of the commenter's favorite team,
  //and then moves all the comments up, just like in a chat room, leaving the bottom cell open.
    if (lastCellModified == "G8"){
      var user = Session.getEffectiveUser().getEmail()
     switch (user){
       case "fakeemail1@gmail.com":r.setBackgroundColor("MidnightBlue").setFontColor("LimeGreen");break; 
       case "fakeemail2@gmail.com":r.setBackgroundColor("MidnightBlue").setFontColor("Orange");break;
       case "fakeemail3t@gmail.com":r.setBackgroundColor("Gray").setFontColor("DarkBlue");break;
       case "fakeemail4@gmail.com":r.setBackgroundColor("Orange").setFontColor("DarkBlue");break;
       case "fakeemail5@gmail.com":r.setBackgroundColor("Gray").setFontColor("DarkBlue");break;
       case "fakeemail6@gmail.com":r.setBackgroundColor("Green").setFontColor("Yellow");break;
     }
            ss.getRange("G2").copyFormatToRange(sheet, 7, 15, 1, 1);//sheet, column, columnEnd, row, rowEnd
            ss.getRange("G2").copyValuesToRange(sheet, 7, 15, 1, 1);
            ss.getRange("G3").copyFormatToRange(sheet, 7, 15, 2, 2);
            ss.getRange("G3").copyValuesToRange(sheet, 7, 15, 2, 2);
            ss.getRange("G4").copyFormatToRange(sheet, 7, 15, 3, 3);
            ss.getRange("G4").copyValuesToRange(sheet, 7, 15, 3, 3);
            ss.getRange("G5").copyFormatToRange(sheet, 7, 15, 4, 4);
            ss.getRange("G5").copyValuesToRange(sheet, 7, 15, 4, 4);
            ss.getRange("G6").copyFormatToRange(sheet, 7, 15, 5, 5);
            ss.getRange("G6").copyValuesToRange(sheet, 7, 15, 5, 5);
            ss.getRange("G7").copyFormatToRange(sheet, 7, 15, 6, 6);
            ss.getRange("G7").copyValuesToRange(sheet, 7, 15, 6, 6);
            ss.getRange("G8").copyFormatToRange(sheet, 7, 15, 7, 7);
            ss.getRange("G8").copyValuesToRange(sheet, 7, 15, 7, 7);
            ss.getRange("G8").clearContent().clearFormat;

    }

  }

SO, that's my request. Some specific things I'm interested in: 1) Am I doing things efficiently? I don't care how fast the code runs (within reason) but I do want to know if I'm wasting a lot of my own time typing needless repetitions of code.

2) It would really have helped me to take some of these little sections out of the main function, creating new functions and inserting calls to them. That would have helped me keep parentheses straight, and likely would have sped up the process of bug-fixing quite a bit. However, I kept having problems when I tried.

---

One more thing:

You may have noticed that there is a problem with the way I am calculating eliminations-- it assumes that if a player could beat all other players in a one-on-one contest, given that everything goes exactly right, then the player is still alive. In fact, though, it is often impossible for a player to have something happen that is the best possible outcome against multiple opponents at once.

(Example: Player has 5 points, Opponent1 has 3 points, Opponent2 has 5 points. One game remains, A vs B. Player picks A for 1 point. Opponent1 picks A for 4 points; Opponent2 picks B for 1 point. Individually, Player can beat Opponent1 if A loses, since he will finish with 5 and Opponent1 will finish with 3. And he can beat Opponent2 if A wins, because he will finish with 6 and Opponent2 will finish with 5. However, if A loses, then Opponent2 will actually get 1 point and win; if A wins, Opponent1 will get 4 points while Player will only get 1 point, finishing with 6 vs Opponent1's 7 points. So in reality, Player is already eliminated.)

So that's the problem I'm working on right now. And I wanted to kill 2 birds with 1 stone, so while checking for these scenarios where a player can't take on both opponents at once, I figured it would also be fun to highlight (in green) all games that a player MUST WIN to remain alive, as well as games that he MUST LOSE to remain alive (red), and of course games that he has to simultaneously win and lose, which cause him to be eliminated (highlighted in black).

Unfortunately, there's only one week left in the season, and I'm not on pace to finish :( So if you want to look over that code as well, you're more than welcome-- it's here on this spreadsheet, you just need to go to "tools->script editor."

Thank you very much for your time!

Answer 1

You can get rid of repetition in several places.

The long if statement could be replaced with this:

if (lastCellModified.indexOf(['A10', 'A11', 'A12', 'A13', 'A14', 'A15', 'A16', 
    'A17', 'A18', 'A19', 'A20', 'A21', 'A22', 'A23', 'A25']) > -1) {

or this:

if (lastCellModified.length == 3 && lastCellModified >= "A10" && lastCellModified <= "A25")

Setting variables for players and opponents can be reduced to:

var pScore = sheetArray[player][1];
var leftPCol = player % 5 + 1;
var rightPCol = 13 - leftPCol;
var pElimRow = player;

The list of sheet.getRange('D2') etc. could be replaced with a loop:

for (var i = 1; i < 6; i++) {
    sheet.getRange("D" + (i + 1)).setValue(sheetArray[i][3]);
}

and similarly for the ss.getRange... list:

for (var i = 1; i < 8; i++) {
    var range = ss.getRange('G' + (i + 1));
    range.copyFormatToRange(sheet, 7, 15, i, 1);
    range.copyValuesToRange(sheet, 7, 15, i, 1);
}

See the full code with a few other changes here

EDIT: I looked at this in a bit more detail. Ultimately what I think you need is to go through every possible combination of outcomes for the remaining games, and check who is the winner under each scenario, and eliminate the players who are not winners under any scenarios. (There might be some shortcut method, but as you have already noted, your solution of comparing pairs of players does not quite do it.) This does involve looking at 2 to the power of N scenarios where N is the number of remaining games. That should be manageable for, say, 16 games, although it may get a little slow. I've altered the script to do this here but couldn't figure out how to test it.

Whether you use this or not, I recommend using the first part of that code, which reads the players' current scores, their picks, and their bets, into arrays with meaningful names at the beginning. This simplifies the rest of the code a lot as you can deal with those arrays instead of having to keep thinking about what reference you need in the spreadsheet.

Answer 2

Few general notes on the code

1. The method onEdit is doing too many things which is making the code difficult to read. I would prefer to break the entire function and distribute responsibilities. The structure would look something like this (this is just an example) -

   var Game = {};
   
   Game.setStatus = {
     validateInput: function () {
     },
     isLastCallModified: function() {
     },
     update: function() {
     },
     moveBox: function() {
     }
   };
   

2. you have check for boolean values in this way if (sameTeam == true) and if (sameTeam == false) which is not the right way. It should be if (sameTeam) and if (!sameTeam)

Answer 3

for readability replace

var ss = ... ;
var sheet = ... ;
var sh = ... ;
var r = ... ;
var user = ... ;
var lastCellModified = ... ;

with

var ss = ... ,
    sheet = ... ,
    sh = ... ,
    r = ... ,
    user = ... ,
    lastCellModified = ... ,