3
\$\begingroup\$

This is pretty much the first actual (not "hello world") program I'm writing. It's a rewrite of this app and my aim was to increase the fetch speed, and increase the cache storage efficency. I've more or less done this through multiple simultaneous fetches and stripping and compressing the game data.

document.getElementById("btn1").addEventListener("click", function () {fetchOGS(document.getElementById("uname").value); });

if (location.hash.split("=")[0] === "#username") {
    fetchOGS(location.hash.split("=")[1].split("&")[0]);
}

var LZString; //Comes from a compression library
var fdata = {}; //Final processed data that is to be graphed

function fetchOGS(user) {
    var username;
    var userID;
    var udata = Number.isNaN(parseInt(user)) ?
        fetch("https://online-go.com/api/v1/players/?username__icontains=" + user + "&format=json"):
        fetch("https://online-go.com/api/v1/players/?id__icontains=" + user + "&format=json");

    udata
        .then(jsonHttpHandler)
        .then(
            (response) => {
                console.log(response);
                username = response.results[0].username;
                userID = response.results[0].id;
                fdata[username] = {};
                fdata[username].user = response.results[0];
                return username;
            })
        .then(
            (username) => {
                console.log("Reading Local Storage...");
                fdata[username] = localStorage.getItem(username) === null?
                    console.log("Cache miss")||{}:
                    JSON.parse(LZString.decompressFromUTF16(localStorage.getItem(username)));
                //Initialize data arrays, if they aren't initialized yet
                fdata[username]=fdata[username]?fdata[username]:{};
                fdata[username].game=fdata[username].game?fdata[username].game:[];                    fdata[username].score=fdata[username].score?fdata[username].score:[];

                appendLocalStorage(userID);
            });
    function jsonHttpHandler(response) {
        if (!response.ok) {
            var error = new Error(response.statusText);
            error.response = response;
            throw error;
        }
        return response.json();
    }
    function appendLocalStorage(userID) {
        console.log("Freshening Local Storage cache");
        var gameFetchErrors = (error) => console.log("Fetch error " + error);
        var pagesToFetch = 0;
        fetch("https://online-go.com/api/v1/players/" + userID + "/games/?format=json&ended__isnull=false&annulled=false&ordering=-ended")
            .then(jsonHttpHandler)
            .then(sortResults)
            .then(recordTime)
            .then((data) => {
                console.log("Fetching more pages");
                pagesToFetch = Math.ceil((data.count - fdata[username].game.length /*games already fetched*/) / 25 /*games per page*/);
                return pagesToFetch;
            })
            .then(fetchMorePages)
            .catch(gameFetchErrors);
        function fetchMorePages(pagesToFetch) {
            console.log(pagesToFetch + " pages left");
            for (var i = 1;i++<=pagesToFetch;) {
                console.log("Fetching page " + i);
                fetch("https://online-go.com/api/v1/players/" + userID + "/games/?format=json&ended__isnull=false&annulled=false&ordering=-ended&page=" + i)
                    .then(jsonHttpHandler)
                    .then(sortResults)
                    .catch(gameFetchErrors);
            }
            return fdata;
        }
        function sortResults (response) {
            var pageLength = response.results.length;
            for (var i = 0; i < pageLength; i++) {
                var gameID = response.results[i].id;
                if (fdata[username].game.includes(gameID)){
                    console.log("Already added game " + gameID + " results. Skiping Iteration");
                    continue;
                }
                fdata[username].game.push(gameID);nament.push(true);

    // Score is either number + " points", "Timeout" or "Resignation"
    //This converts to a compact form, number, "T" or "R" respectively

                fdata[username].score.push(
                    Number.isNaN(parseInt(response.results[i].outcome[0])) ?
                        response.results[i].outcome.split(" ")[0] :
                        response.results[i].outcome[0]
                );
            }
            if (response.count === fdata[username].game.length) {
                localStorage.setItem(username, LZString.compressToUTF16(JSON.stringify(fdata[username])));
                console.log("Saved game records to local storage");
            }
            return response;
        }
    }
}

All in all, I'm very happy with the performance of the fetching stage, which went from ~40 s to ~6 s, and the stripping and compression, which brought the data size down from ~1.4 MB to ~7 KB (using test user "timuzhti"), but I was wondering if there were any ways to speed up the sorting stage or neaten the code a little.

\$\endgroup\$
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Vogel612 Jan 19 '17 at 10:21
4
\$\begingroup\$

I'm not a JavaScript expert, and overall it looks good, but there are a few things I would change (mostly style):

fdata[username].game.push(gameID);nament.push(true);

I almost missed that nament.push(true); call, let's break that to a new line. You can always run a minify-script later to shrink the byte-size.


I would alter this slightly:

var udata = Number.isNaN(parseInt(user)) ?
    fetch("https://online-go.com/api/v1/players/?username__icontains=" + user + "&format=json"):
    fetch("https://online-go.com/api/v1/players/?id__icontains=" + user + "&format=json");

It's hard to read at the moment, a lot goes on:

var fetchParam = Number.isNaN(parseInt(user)) ? "username" : "id";
var udata =
    fetch("https://online-go.com/api/v1/players/?" + fetchParam + "__icontains=" + user + "&format=json");

It also shrinks the byte-size, which is good.


fdata[username].game=fdata[username].game?fdata[username].game:[];                    fdata[username].score=fdata[username].score?fdata[username].score:[];

Why is that a single line? Let's break it up so that we don't miss that bit that gets hidden by scrolling.


Every single URL call starts with:

https://online-go.com/api/v1/players/

And contains:

format=json

Let's encapsulate those to variables and use them:

var urlBase = "https://online-go.com/api/v1/players/";
var format = "format=json";

var fetchParam = Number.isNaN(parseInt(user)) ? "username" : "id";
var udata = fetch(urlBase + "?" + fetchParam + "__icontains=" + user + "&" + format);

We can avoid repetition here:

fdata[username].score.push(
    Number.isNaN(parseInt(response.results[i].outcome[0])) ?
        response.results[i].outcome.split(" ")[0] :
        response.results[i].outcome[0]
);

To:

var outcome = response.results[i].outcome;
fdata[username].score.push(
    Number.isNaN(parseInt(outcome[0])) ? outcome.split(" ")[0] : outcome[0];
};

Shorter, sweeter and more maintainable.


Overall, excellent start. I hope someone who is much more acquainted with JavaScript than I comes along and gives you more advice. :)

\$\endgroup\$
  • \$\begingroup\$ Thanks for the tips. The "nament" bit was just a typo I missed. I've removed it now XD. I guess I should use variables more. \$\endgroup\$ – timuzhti Jan 19 '17 at 10:18

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.