patternjavascriptMinor
Script for keeping track of teams in a class
Viewed 0 times
scriptkeepingtrackteamsforclass
Problem
I wrote this script to help me keep track of who has worked together and for what project or discussion in my classes.
I make a Google Sheet with the names of the students going across the first row and down the first column. Each student gets a number corresponding to their location in the spreadsheet. The value is the letter or number to represent the project or discussion and the numbers are the students who are on a team for that project.
I was particularly unsure of my use of
```
function fillInTeam(value, numbers){
var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
var first = 1;
for (var i = 0; i< numbers.length; i++) {
var theNum1 = numbers[i];
for (var z = first; z < numbers.length; z++) {
var y = numbers[z];
var cell = spreadsheet.getDataRange().getCell(theNum1, y).setBackground('yellow');
cell.setValue(cell.getValue() + value);
}
first++;
}
}
function updateTeams(){
clearBackgrounds();
fillInTeam('s', [15, 14]);
fillInTeam('s', [13, 12]);
fillInTeam('s', [11,
I make a Google Sheet with the names of the students going across the first row and down the first column. Each student gets a number corresponding to their location in the spreadsheet. The value is the letter or number to represent the project or discussion and the numbers are the students who are on a team for that project.
I was particularly unsure of my use of
getDataRange() because it seemed like that might not really be necessary since I'm just trying to get a cell. I just didn't see a more efficient way to do that. In my function for setting the backgrounds of the cells to white, I tried using getRange('upperleft:lowerright'), but I wasn't sure if that was really any better (and could possibly introduce errors if I tried to reuse the code for similar purposes in a larger class since I might forget to change it). The updateTeams() code is just where I enter the teams to see how that balances things out since I am trying to have students pair up with as many people as possible in the first few discussions or projects. It's there so I can try out different arrangements without having to remember much about what I tried before. If there is a yellow square at the intersection of two names, it means they have worked together before and the letters or numbers tell on what they worked together.```
function fillInTeam(value, numbers){
var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
var first = 1;
for (var i = 0; i< numbers.length; i++) {
var theNum1 = numbers[i];
for (var z = first; z < numbers.length; z++) {
var y = numbers[z];
var cell = spreadsheet.getDataRange().getCell(theNum1, y).setBackground('yellow');
cell.setValue(cell.getValue() + value);
}
first++;
}
}
function updateTeams(){
clearBackgrounds();
fillInTeam('s', [15, 14]);
fillInTeam('s', [13, 12]);
fillInTeam('s', [11,
Solution
- I like your argument names for
fillInTeam. I think they were good choices.
- You declare the variable
firstand only use it here.
for (var z = first; z < numbers.length; z++) {You can just set
z = i + 1 instead.for (var z = i + 1; z < numbers.length; z++) {- Speaking of, use
jinstead ofz. Loop counters traditionally start withiand work their way up the alphabet as nesting goes deeper.
-
theNum1 and y are both absolutely atrocious names. Find something better and more meaningful for them, or better yet, just get rid of them entirely. Just use i and j to access the array elements directly.function fillInTeam(value, numbers){
var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
for (var i = 0; i< numbers.length; i++) {
for (var j = i + 1; j < numbers.length; j++) {
var cell = spreadsheet.getDataRange().getCell(numbers[i], numbers[j]).setBackground('yellow');
cell.setValue(cell.getValue() + value);
}
}
}-
getDataRange().getCell() was the right choice here. getRange() is your other option, but it only takes A1 notation. That would have been less flexible in the long run. -
Getting the DataRange inside of the loop is a bit inefficient. Instead of declaring
spreadsheet as a variable, declare a dataRange variable and use it to access the individual cells.function fillInTeam(value, numbers){
var dataRange = SpreadsheetApp.getActiveSpreadsheet().getDataRange();
for (var i = 0; i< numbers.length; i++) {
for (var j = i + 1; j < numbers.length; j++) {
var cell = dataRange.getCell(numbers[i], numbers[j]).setBackground('yellow');
cell.setValue(cell.getValue() + value);
}
}
}-
One last transformation.
setBackground() returns a cell, so you can chain the calls together like this. (Note that I like chaining, but I might have taken it too far and made it less clear by getting rid of the cell variable.)function fillInTeam(value, numbers){
var dataRange = SpreadsheetApp.getActiveSpreadsheet().getDataRange();
for (var i = 0; i< numbers.length; i++) {
for (var j = i + 1; j < numbers.length; j++) {
dataRange.getCell(numbers[i], numbers[j])
.setBackground('yellow')
.setValue(cell.getValue() + value);
}
}
}updateTeams() has a definite smell to it, but I suspect that's because you're manually changing those prior to each run. I'm not sure what to do about it though. clearBackgrounds() is a little clunky. It could definitely benefit from a spreadsheet variable like you have in fillInTeam(). I also don't like this.
.getRange('B2:O15').setValue('');What happens if you add columns to your spreadsheet beyond
O? You'll have to update your code to handle it. I'm thinking you can use getLastRow and offset to do this instead. Something like this untested code here.var dataRange = spreadsheet.getDataRange();
dataRange.offset(1,0,dataRange.getLastRow() - 1, dataRange.getLastColumn()).setValue('');I'm admittedly unhappy with how far it scrolls, but it's dynamic at least.
Code Snippets
for (var z = first; z < numbers.length; z++) {for (var z = i + 1; z < numbers.length; z++) {function fillInTeam(value, numbers){
var spreadsheet = SpreadsheetApp.getActiveSpreadsheet();
for (var i = 0; i< numbers.length; i++) {
for (var j = i + 1; j < numbers.length; j++) {
var cell = spreadsheet.getDataRange().getCell(numbers[i], numbers[j]).setBackground('yellow');
cell.setValue(cell.getValue() + value);
}
}
}function fillInTeam(value, numbers){
var dataRange = SpreadsheetApp.getActiveSpreadsheet().getDataRange();
for (var i = 0; i< numbers.length; i++) {
for (var j = i + 1; j < numbers.length; j++) {
var cell = dataRange.getCell(numbers[i], numbers[j]).setBackground('yellow');
cell.setValue(cell.getValue() + value);
}
}
}function fillInTeam(value, numbers){
var dataRange = SpreadsheetApp.getActiveSpreadsheet().getDataRange();
for (var i = 0; i< numbers.length; i++) {
for (var j = i + 1; j < numbers.length; j++) {
dataRange.getCell(numbers[i], numbers[j])
.setBackground('yellow')
.setValue(cell.getValue() + value);
}
}
}Context
StackExchange Code Review Q#61525, answer score: 4
Revisions (0)
No revisions yet.