Problem
I have the following Greasemonkey script that fetches some data from a Yahoo webpage and injects it into a webpage. It has the feature that it only fetches the data once per day instead of fetching it every single time the page is loaded.
I think my displayresults function is a little messy just because it has to access a lot of variables to work.
You can test this script for yourself by installing it and loading this page.
// ==UserScript==
// @name Yahoo Fantasy Football Rank
// @author Bijan
// @version 2.5
// @description Very simple script to conveniently see how many points a team gives up
// @namespace http://albuyeh.com
// @match *://football.fantasysports.yahoo.com/*
// @require http://ajax.googleapis.com/ajax/libs/jquery/2.1.0/jquery.min.js
// @grant GM_xmlhttpRequest
// @grant GM_getValue
// @grant GM_setValue
// ==/UserScript==
//Map Team Name to abbreviation
var teamNameList = {
"Arizona Cardinals": "Ari",
"Atlanta Falcons": "Atl",
"Baltimore Ravens": "Bal",
"Buffalo Bills": "Buf",
"Carolina Panthers": "Car",
"Chicago Bears": "Chi",
"Cincinnati Bengals": "Cin",
"Cleveland Browns": "Cle",
"Dallas Cowboys": "Dal",
"Denver Broncos": "Den",
"Detroit Lions": "Det",
"Green Bay Packers": "GB",
"Houston Texans": "Hou",
"Indianapolis Colts": "Ind",
"Jacksonville Jaguars": "Jax",
"Kansas City Chiefs": "KC",
"Miami Dolphins": "Mia",
"Minnesota Vikings": "Min",
"New England Patriots": "NE",
"New Orleans Saints": "NO",
"New York Giants": "NYG",
"New York Jets": "NYJ",
"Oakland Raiders": "Oak",
"Philadelphia Eagles": "Phi",
"Pittsburgh Steelers": "Pit",
"San Diego Chargers": "SD",
"San Francisco 49ers": "SF",
"Seattle Seahawks": "Sea",
"St. Louis Rams": "StL",
"Tampa Bay Buccaneers": "TB",
"Tennessee Titans": "Ten",
"Washington Redskins": "Was",
}
const playerPositions = ["QB", "WR", "RB", "TE", "K", "DEF"]; //Positions to fetch data for
const numPositions = playerPositions.length;
const baseURL = "http://football.fantasysports.yahoo.com/f1/326198/pointsagainst?pos="; //Page to create fetch request to
var rankingsTable = {}; //This will be filled by the AJAX parser.
var final = {}; //Final table that holds all of the team/position info
var numPagesFetched = 0;
var tableName = "FF_Array2";
//Display notification of Update
var ver = GM_info.script.version;
if (GM_getValue("version", "") < ver) {
GM_setValue("version", ver);
alert("Updated Yahoo Fantasy Football Rank by Bijan to v" + ver);
}
//Fetch results once per day.
var now = new Date();
var currdate = Math.floor(now/8.64e7);
if (GM_getValue("date2", "") < currdate || GM_getValue (tableName, "").length == 0) {
GM_setValue("date2", currdate);
console.log("Fetched results @ :" + currdate);
main(); //Go to main() to fetch data
}
else {
console.log("Already fetched data. Continuing to display results");
displayResults();
}
function main () {
for (var J in playerPositions) {
GM_xmlhttpRequest ( {
method: "GET",
url: baseURL + playerPositions[J],
context: playerPositions[J],
onload: parseResponse,
onerror: function (e) { console.error ('**** error ', e); },
onabort: function (e) { console.error ('**** abort ', e); },
ontimeout: function (e) { console.error ('**** timeout ', e); }
} );
}
}
function parseResponse (response) {
var playerPosition = response.context;
var parser = new DOMParser ();
var ajaxDoc = parser.parseFromString (response.responseText, "text/html");
var statRows = ajaxDoc.querySelectorAll ("#statTable0 > tbody > tr");
var newStatTable = $(statRows).map ( function () {
var tblRow = $(this);
var teamRank = parseInt (tblRow.find (".rank-indicator").text().trim(), 10);
var teamName = teamNameList[tblRow.find ("td:eq(1)").text().trim().split(" vs")[0]];
return [ [teamName, teamRank] ];
} ).get ();
numPagesFetched++;
console.log ('Fetched page ' + numPagesFetched + ' of ' + numPositions + '.');
/*--- Now loop over the fetched rows and collate them into the master table, depending
on playerPosition.
*/
var columnIdx = playerPositions.indexOf (playerPosition);
for (var K in newStatTable) {
var teamName = newStatTable[K][0];
var teamRank = newStatTable[K][1];
var teamStats = rankingsTable[teamName] || new Array (numPositions);
teamStats[columnIdx] = teamRank;
rankingsTable[teamName] = teamStats;
}
if (numPagesFetched === numPositions) {
displayFinalResult ();
}
}
function displayFinalResult () {
//Sort team name
var sortedTeamNames = Object.keys (rankingsTable).sort ( function (zA, zB) {
return zA.localeCompare (zB);
} );
//Store team name and rank array in a single array
for (var J in sortedTeamNames) {
var teamName = sortedTeamNames[J];
if (rankingsTable.hasOwnProperty (teamName) ) {
final[teamName] = rankingsTable[teamName]
}
}
//Save array to browser
GM_setValue (tableName, JSON.stringify (final) );
displayResults(final)
}
function displayResults(result) {
//Get saved results
var myList = {};
var myListObj = GM_getValue (tableName, "");
if (myListObj) {
myList = JSON.parse (myListObj);
}
//Check if we have already fetched results for the day
if (typeof result === 'undefined') { result = myList; console.log("Loading pre-loaded data");}
var player = document.querySelectorAll('span[class="Fz-xxs"]'); //Get player element
var opp = document.querySelectorAll('a[class="F-reset"]'); //Get opposing team element
if(opp[0].innerHTML == "View Details") opp = Array.prototype.slice.call(opp, 1, opp.length);
if(player.length == opp.length) {
console.log("Player length = Opposing Team length. We can continue")
for (i=0; i < player.length; i++) {
var playerPos = player[i].innerHTML.split(" - ").splice(-1)[0]; //Returns position (i.e. QB)
var playerIndex = playerPositions.indexOf(playerPos);
if (playerIndex == -1) {console.log("Skipped " + playerPos); continue;} //Currently does not fetch Defensive players.
var oppTeam = opp[i].innerHTML.split(" ").splice(-1)[0]; //Returns opposing team (i.e. SF)
var rank = result[oppTeam][playerIndex]; //Get rank # based on opposing team and player position
if (1 <= rank && rank <= 10) color = "F-rank-good"; //Green
else if (11 <= rank && rank <= 22) color = "F-rank-neutral"; //Yellow
else if (23 <= rank && rank <= 32) color = "F-rank-bad"; //Red
console.log(oppTeam + " gives up the #" + rank + " points to the " + playerPos + " position")
opp[i].innerHTML = opp[i].innerHTML.split(/@|vs/)[0] + " @ <span class='" + color + "'>" + oppTeam + " - " + rank + "</span>"; //Modify element to include rank and color
}
}
else {
console.log("Player & Team mismatch: " + player.length + " & " + opp.length)
}
}
// End
Solution
Consider storing large data in a separate file
teamNameList is quite long without necessarily providing any information for readers of the code. Why not save it in a separate file, say data.js and include it that way? You can read this SO post that explains how to include local files when working with GreaseMonkey.
Avoid loose statements that sort of run when the page loads without being explicit about it
What I mean by this is you usually want to avoid JavaScript statements that aren’t tied to an event. Sure you want to run these when the page loads, right? In that case make this explicit like so:
document.onload = function ...
and put everything you want to happen in there. This will save you from the spaghetti code effect of not quite knowing why or when something is happening. Your code will be much more maintainable if you have everything that should happen on page load in one place. Otherwise, when you are debugging, things could get ugly. Read more about this in this SO post.
So I’d recommend, for example, moving this entire unanchored chunk into some kind of onload function:
//Fetch results once per day.
var now = new Date();
var currdate = Math.floor(now/8.64e7);
if (GM_getValue("date2", "") < currdate || GM_getValue (tableName, "").length == 0) {
GM_setValue("date2", currdate);
console.log("Fetched results @ :" + currdate);
main(); //Go to main() to fetch data
}
else {
console.log("Already fetched data. Continuing to display results");
displayResults();
}
Variable Names
I agree with @Malachi that your for-loop variable names (K, J) could be improved by naming them something related to what they are indexing. Also you’d usually want to use a lower-case letter for a for-loop variable, not an uppercase. Use upper case and people wonder if they are missing something…
Formatting
displayResults is kind of a beast. Even without rewriting anything, I prefer it with some spacing to make it more comprehensible:
function displayResults(result) {
//Get saved results
var myList = {};
var myListObj = GM_getValue (tableName, "");
if (myListObj) {
myList = JSON.parse (myListObj);
}
//Check if we have already fetched results for the day
if (typeof result === 'undefined') { result = myList; console.log("Loading pre-loaded data");}
//Comment here would be nice
var player = document.querySelectorAll('span[class="Fz-xxs"]'); //Get player element
var opp = document.querySelectorAll('a[class="F-reset"]'); //Get opposing team element
//Comment here would be nice
if(opp[0].innerHTML == "View Details") opp = Array.prototype.slice.call(opp, 1, opp.length);
if(player.length == opp.length) {
console.log("Player length = Opposing Team length. We can continue")
for (i=0; i < player.length; i++) {
var playerPos = player[i].innerHTML.split(" - ").splice(-1)[0]; //Returns position (i.e. QB)
var playerIndex = playerPositions.indexOf(playerPos);
//Comment here would be nice
if (playerIndex == -1) {console.log("Skipped " + playerPos); continue;} //Currently does not fetch Defensive players.
var oppTeam = opp[i].innerHTML.split(" ").splice(-1)[0]; //Returns opposing team (i.e. SF)
var rank = result[oppTeam][playerIndex]; //Get rank # based on opposing team and player position
if (1 <= rank && rank <= 10) color = "F-rank-good"; //Green
else if (11 <= rank && rank <= 22) color = "F-rank-neutral"; //Yellow
else if (23 <= rank && rank <= 32) color = "F-rank-bad"; //Red
console.log(oppTeam + " gives up the #" + rank + " points to the " + playerPos + " position")
opp[i].innerHTML = opp[i].innerHTML.split(/@|vs/)[0] + " @ <span class='" + color + "'>" + oppTeam + " - " + rank + "</span>"; //Modify element to include rank and color
}
}
else {
console.log("Player & Team mismatch: " + player.length + " & " + opp.length)
}
}
Use more DOM access methods
If you use more DOM or jQuery you can shorten a lot of lines of code here, such as the last line in your for-loop in displayResults. It’s not just about safety or what have you (I don’t know much about why people hate innerHTML so much)…it’s also about readability. Some of those lines get awfully long when you throw in <span> tags and all the rest in your JavaScript code.
Questions
- In
parseResponsewhy do youreturn [[teamName, teamRank]]instead
of justreturn [teamName, teamRank]? - Did I miss your error handling apart from
main()? What actually happens if your data retrieval fails?
Right here
//Store team name and rank array in a single array
for (var J in sortedTeamNames) {
var teamName = sortedTeamNames[J];
if (rankingsTable.hasOwnProperty (teamName) ) {
final[teamName] = rankingsTable[teamName]
}
}
This is messy, you create a variable J to iterate through the objects of sortedTeamNames and then use it for an index to set the team name. I am getting confused just trying to put that into words.
You should make the iteration variable teamName instead of J and use it through the loop, but in order to do this you need to make this a for each loop, but it appears that for each … in is now deprecated and you should instead use for…of
for (var teamName of sortedTeamNames) {
if (rankingsTable.hasOwnProperty (teamName) ) {
final[teamName] = rankingsTable[teamName]
}
}
It should work exactly the same.