1
\$\begingroup\$

I'd like to know if this is the right syntax for using cookies. I haven't yet tested this code with cookies being used.

I'm using a jQuery Plugin called jPages to display pages on my (one page) website, it has a lot of options and it's very flexible.

The jPages plugin has an option to start from a different page per say, and a callback function to find what page you are currently on.

I'm using localStorage to store the last page that a visitor has been on so that it can be retrieved when they return. I've tested this method and it works perfectly.

I'm using Modernizr to test for localStorage support in browsers, and if localStorage is not supported I use cookies to try and achieve the same experience.

Here's the part of my code that does that:

var set_data = function(cluster) {
    if (!localStorage.getItem("Page")) {
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    } else {
        localStorage.removeItem("Page");
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    }
}

var get_data = function() {
    if (localStorage.getItem("Page")) {
        var last_page = JSON.parse(localStorage.getItem("Page"))
        log("Last Page: " + last_page);
        return last_page;
    } else {
        log("Last Page: Inexistent");
        return 1;
    }
}

var set_last_page = function(cluster) {
    if (Modernizr.localstorage) {
        var data = cluster;
        set_data(data);
    } else {
        var data = JSON.stringify(cluster);
        create_cookie("Page", data, 365);
    }
}

var get_current_page = function() {
    if (Modernizr.localstorage) {
        var data = get_data();
    } else {
        var data = JSON.parse(read_cookie("Page"));
    }
    return data;
}

var create_cookie = function(name, value, days) {
    var date;
    var expires;

    if (days) {
        date = new Date();
        date.setTime(date.getTime() + (days * 24 * 60 * 60 * 1000));
        expires = "; expires=" + date.toGMTString();
    } else {
        expires = "";
    }
    document.cookie = name + "=" + value + expires + "; path=/";
}

var read_cookie = function(name) {
    var nameEQ = name + "=";
    var ca = document.cookie.split(';');
    var i;
    var c;

    for (i = 0; i < ca.length; i++) {
        c = ca[i];
        while (c.charAt(0) == ' ') {
            c = c.substring(1, c.length);
        }

        if (c.indexOf(nameEQ) == 0) {
            return c.substring(nameEQ.length, c.length);
        }
    }
    return null;
}

The localStorage part works, but I'm not sure if I'm doing the cookie part correctly.

If someone can tell me if this is the right syntax for using cookies, I would appreciate it.

\$\endgroup\$
1
\$\begingroup\$

At the expense of having a little more code, I would use a localStorage polyfill [2].

With that code in place you no longer need to keep the create and read cookie methods. You also don't need the code wrapping get_data and set_data. Your resulting code would look something like this:

set_last_page = function(cluster) {
    if (!localStorage.getItem("Page")) {
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    } else {
        localStorage.removeItem("Page");
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    }
}

get_current_page = function() {
    if (localStorage.getItem("Page")) {
        var last_page = JSON.parse(localStorage.getItem("Page"))
        log("Last Page: " + last_page);
        return last_page;
    } else {
        log("Last Page: Inexistent");
        return 1;
    }
}

From here I would make a few changes:

  1. You don't need to check for existence or remove an item from storage to change it.
  2. The logging code is unnecessary.
  3. Using JSON.stringify and JSON.parse is considerable overkill for converting between a string and a number.

Taking care of these would leave me with this code:

set_last_page = function(cluster) {
    localStorage.setItem("Page", cluster+'');
}

get_current_page = function() {
    return +(localStorage.getItem("Page") || 1); 
    //localStorage.getItem("Page") will return either 
    // a number >= 1 (as a string), undefined or null 
    //so '|| 1' will take care of the latter cases
}

Finally, I dislike the fact you are initializing these variables as function expressions (statements) and don't end them with semicolons (either add the semis or make them function declarations). I would also rename things to be more consistent and in line with my preferred naming conventions for javascript:

function setPage(page) {
    localStorage.setItem('page', page+'');
}

function getPage() {
    return +(localStorage.getItem('page') || 1); 
}
\$\endgroup\$
  • \$\begingroup\$ I did not knew that the localStorage behaves the same way as cookies when it comes to overwrite, so this means I do not need to remove it each time I have a new values for it ? I'm doing the logging because I'm testing now and I like seeing in the console if there's something happening, after I'm done with the testing I remove all the console outputs. And about JSON, I've read that it's best to use it, as the polyfill does too. And I like having the variables as expressions rather than just simple functions :) But that's just me I guess. Thanks for the response, I appreciate it \$\endgroup\$ – Roland May 4 '12 at 19:15
  • \$\begingroup\$ Oh, so, if I use the polyfill, I'll end up using only those last two functions by your description, right ? \$\endgroup\$ – Roland May 4 '12 at 19:18
  • \$\begingroup\$ @Roland, correct (you only nee those last two functions, and it behaves the same way as cookies) \$\endgroup\$ – Bill Barry May 4 '12 at 21:52

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.