5
\$\begingroup\$

In a current javascript project, I'm working with the browsers localStorage, and it will pretty consistently be full. To overcome this, I wrote a wrapper to surround the localStorage object that would keep track of when an object was added.

The hard(er) part, was coming up with the algorithm to make room for any new data. I was wondering if anyone could critique my method, and let me know if a better method exists.

DataStore.prototype.makeRoom = function() {
    var tmpData = {};
    for (var key in localStorage) {
        var expTime = JSON.parse(localStorage.getItem(key)).expirationTime;
        if (Object.size(tmpData) < 3) {
            tmpData[key] = expTime;
            continue;
        }
        for (var tmpKey in tmpData) {
            var tmpExp = JSON.parse(localStorage.getItem(tmpKey)).expirationTime;
            if (tmpExp > expTime) {
                delete tmpData[tmpKey];
                tmpData[key] = expTime;
                break;
            }
        }
    }
    for (var deleteKey in tmpData) {
        this.destroyItem(deleteKey);
    }
    return true;
};

And for reference, DataStore.destroyItem(), and DataStore.store() where the data is written:

DataStore.prototype.store = function(dataKey, data, minutesUntilExpiration, overWrite) {
    if (!overWrite && this.hasItem(dataKey))
        return false;
    var dataToSave = {
        data: data,
        expirationTime: (new Date().getTime() + (minutesUntilExpiration * 60 * 1000))
    };
    try {
        localStorage.setItem(dataKey, JSON.stringify(dataToSave));
    } catch (e) {
        this.makeRoom();
        this.store(dataKey,data,minutesUntilExpiration,overWrite);
    }
    return true;
};

DataStore.prototype.destroyItem = function(dataKey) {
    if (this.hasItem(dataKey)) {
        localStorage.removeItem(dataKey);
        return true;
    }
    return false;
};

Object.size = function(obj) {
    var size = 0, key;
    for (key in obj) {
        if (obj.hasOwnProperty(key)) size++;
    }
    return size;
};
\$\endgroup\$
4
  • \$\begingroup\$ How many items do you expect to manage in localStorage ( maximum ) ? \$\endgroup\$
    – konijn
    Commented Jan 2, 2014 at 20:18
  • \$\begingroup\$ Wouldn't you rather delete items that are past their expiration time before you start ejecting items arbitrarily? \$\endgroup\$ Commented Jan 2, 2014 at 20:54
  • \$\begingroup\$ tom, between 50-150 have been my observations before it has to start deleting \$\endgroup\$
    – Nathan
    Commented Jan 2, 2014 at 21:31
  • \$\begingroup\$ 200_success, Ideally, but under consistent use for say 5-10 minutes, the cache will have already filled. The data is essentially good forever so the only logical way in my mind was to start deleting the oldest. I'm primarily using it so when the user clicks the back button, it only needs to make 1 ajax call, rather than 20 that have to be spaced a second apart. \$\endgroup\$
    – Nathan
    Commented Jan 2, 2014 at 21:32

1 Answer 1

5
\$\begingroup\$

Object.size

First off, if it's ok with you to use ES5 APIs (you use localStorage, it should be okay), then here's a quicker way to get the number of keys in an object using Object.keys.

return Object.keys(obj).length

Delete by N

Also, from what I understand in your code, you're deleting keys by 3's. If so, then you should not hardcode the 3. Instead, have it configurable in your object.

Verbose Variable Naming

Here's one I get often. Name variables according to use. tmpExp will not help anyone understand its purpose. Name them verbosely. I should understand code like I read the back of a milk box or something.

Compress the algorithm

I suggest you compress the algorithm, make it simple.

Nested loops are just bad for performance. Also, getItem is a synchronous operation, which means that if it freezes, it freezes the UI as well. Nesting that along with a JSON.parse makes it even more slow.

If you can, do operations with the least loop runs and function calls as much as possible. If you can cram several loops into one big loop (like batched operations), then do so.

Here's a proposed fix for makeRoom. Keeping it simple, it just removes the oldest among the stored data.

// Just grab the expiry and store the expiry-key into an object
var expiries = Object.keys(localStorage).reduce(function(collection,key){
  var currentExpirationTime = JSON.parse(localStorage.getItem(key)).expirationTime;
  collection[currentExpirationTime] = key;
  return collection;
},{});

// Get the expiry dates into an array
var expiryDates = Object.keys(expiries);

// For N times, find the oldest (smallest timestamp) and destroy
for(var i = 0; i < N; i++){
  var oldestDate = Math.min.apply(null,expiryDates);
  this.destroyItem(expiries[oldestDate]);
}

getTime now()!

Instead of new Date().getTime(), there's the new Date.now(). Same effect, but saves you memory from creating a new Date instance just to get the current timestamp.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the feedback! That algorithm looks like it should perform much better, I feel a little dumb for not coming up with a more elegant solution. 'Verbose Variable Naming' - absolutely, as sad as it is, this is a major improvement for me, I used to be the king of 1 letter variables. I have seen the light though! \$\endgroup\$
    – Nathan
    Commented Jan 2, 2014 at 21:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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