HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Function to delete oldest items out of HTML5 localStorage. Can I make it more efficient?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
candeleteoldestmakefunctionmoreefficientlocalstorageitemshtml5

Problem

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)  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;
};

Solution

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.

Code Snippets

return Object.keys(obj).length
// 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]);
}

Context

StackExchange Code Review Q#38441, answer score: 5

Revisions (0)

No revisions yet.