4
\$\begingroup\$

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
\$\endgroup\$
  • \$\begingroup\$ Using innerHTML is poor practice and has pitfalls. Search SO for why. Then try to write the code to use DOM methods and/or jQuery and/or textContent but NOT innerHTML and NOT .html(). \$\endgroup\$ – Brock Adams Sep 4 '15 at 4:49
  • \$\begingroup\$ Do not use anything like .length in the conditional part of an if statement. Lookup why on SO. \$\endgroup\$ – Brock Adams Sep 4 '15 at 4:51
  • \$\begingroup\$ Is there a better way to modify the text other than just looping through both the player and opposite team elements? \$\endgroup\$ – Bijan Sep 4 '15 at 4:58
  • \$\begingroup\$ Dunno. Have to spend time analyzing it. Maybe not. \$\endgroup\$ – Brock Adams Sep 4 '15 at 5:16
  • \$\begingroup\$ Why are you using jQuery? half of your DOM access is using standard querySelectorAll, the other half uses jQuery \$\endgroup\$ – Dan Pantry Sep 4 '15 at 19:38
2
\$\begingroup\$

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

  1. In parseResponse why do you return [[teamName, teamRank]] instead of just return [teamName, teamRank]?
  2. Did I miss your error handling apart from main()? What actually happens if your data retrieval fails?
\$\endgroup\$
  • \$\begingroup\$ The .onload part is completely superfluous in this case. GM/TM scripts run at DOMContentLoaded by default and this is the optimum time in 99% of use cases. \$\endgroup\$ – Brock Adams Sep 4 '15 at 19:12
  • \$\begingroup\$ @BrockAdams that's good to know, but wouldn't you still agree with me that it's best to have all the onload-ish type code in one place? Otherwise it's easy to lose track of what runs in what order, etc. \$\endgroup\$ – sunny Sep 4 '15 at 19:15
  • \$\begingroup\$ Sometimes. Not seeing the need in this case. In general userscripts don't need that unless they run at document-start. \$\endgroup\$ – Brock Adams Sep 4 '15 at 19:18
  • \$\begingroup\$ How would I include data.js? Would it just be //@require http://MYSITE/data.js? \$\endgroup\$ – Bijan Sep 4 '15 at 19:41
  • \$\begingroup\$ @Bijan I don't know how to work with Greasemonkey, but yes it looks like your syntax is about right. Check this SO post: stackoverflow.com/questions/13883610/… \$\endgroup\$ – sunny Sep 4 '15 at 20:00
1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ This is clearer by only a trivial amount. The code should not be any faster (haven't tested) but it is one hell of a lot less portable. \$\endgroup\$ – Brock Adams Sep 4 '15 at 18:36
  • \$\begingroup\$ in that case @BrockAdams you would use the Deprecated for each ... in syntax instead, which I mentioned. and this is a user script that is intended for Chrome or Firefox anyway so it is portable in that it works in the places it was meant to work \$\endgroup\$ – Malachi Sep 4 '15 at 19:03
  • 1
    \$\begingroup\$ @BrockAdams this would be correct if transpilers such as babel didn't exist. Fortunately for everyone, they do! \$\endgroup\$ – Dan Pantry Sep 4 '15 at 19:08
  • 1
    \$\begingroup\$ @Malachi, You're right. OP mentioned performance elsewhere, not here. Sorry. \$\endgroup\$ – Brock Adams Sep 4 '15 at 19:16
  • 1
    \$\begingroup\$ @Malachi, My ideas for performance are in the comments to the question. As for making an answer, I've already spent more time now on this than I wished and shall leave you all to it. :) \$\endgroup\$ – Brock Adams Sep 4 '15 at 19:23

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.