Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here is my JavaScript function to manipulate the hash:

var hash = {
    get: function(key){
        var lh = location.hash;

        if(lh.charAt(0) === "#"){
            lh = lh.slice(1);
        }

        var pairs = lh.split("&");
        var obj = [];
        for(var i = 0; i < pairs.length; i++){
            var pair = pairs[i];
            if(pair.indexOf("=") > 0){
                pair = pair.split("=");
                obj[pair[0]] = pair[1];
            }else{
                obj[pair] = 1;
            }
        }
        return typeof key !== 'undefined' ? obj[key] : obj;
    },
    set: function(key, value, cb){
        var newHash = this.removeKey(key);

        if(newHash.length > 1){
            newHash += "&";
        }
        newHash += key + (typeof value != 'undefined' ? "="+value : "");
        var url = location.href.split("#")[0] + (newHash !== "#" ? newHash : "");

        history.pushState(undefined, document.title, url);
        if(cb){
            cb();
        }
    },
    del: function(key){
        var newHash = this.removeKey(key);
        var url = location.href.split("#")[0] + (newHash !== "#" ? newHash : "");

        history.pushState(undefined, document.title, url);
    },
    removeKey: function(key){
        var string = location.hash;

        if(string.charAt(0) !== "#"){
            string = "#" + string;
        }

        var regex = new RegExp("[#&]"+key+"(=[^&]+|)", 'gi');
        string = string.replace(regex, "");

        if(string.charAt(0) === "&"){
            string = "#" + string.slice(1);
        }

        return string !== '' ? string : '#';
    }
};

I am trying to make it as light as possible.

share|improve this question

1 Answer 1

up vote 2 down vote accepted

Streamlined code for get:

get: function(key){
    var lh = location.hash.replace(/^#/, "");
    var pairs = lh.split("&");
    var obj = [], pair;

    for(var i = 0; i < pairs.length; i++){
        pair = pairs[i].split("=");
        if (pair.length === 1) {
            pair.push(1);
        }
        obj[pair[0]] = pair[1];
    }
    return typeof key !== 'undefined' ? obj[key] : obj;
}, 

I also don't see why there's a callback passed to .set()? There's no async operation and the callback is executed at the end of the function with no args so the caller could just put that same code right after the .set() call. There doesn't seem to be any reason for a callback argument.

share|improve this answer
    
That really is a lot better. Thanks a lot. P.S. I took out the callback. I just had that there to test something. –  Marcel Feb 28 at 3:35
    
@Marcel - care to accept this answer? –  jfriend00 May 22 at 0:24

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.