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

Saving data in local storage

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

Problem

I have written a small library for saving data on local storage. I am still a learner and I am not sure whether my code is OK for a production level application.

function AppStorage (aName) {
    "use strict";

    var prefix  = aName;

    var saveItem = function (key, value) {

        if (!key || !value) {

            console.error("Missing parameters \"key\" and \"value\"");    

            return false;
        }

        if (window.localStorage && window['localStorage']) {

            try {

                localStorage.setItem(prefix + '_' + key, JSON.stringify(value));

            } catch (e) {

                return false;

            }

        } else  {

            return false;

        }

    }

    var getItem = function (key) {

        if (!key) {

            console.error("Missing parameter \"key\"");    

            return false;
        }

        if (window.localStorage && window['localStorage']) {
            try {

                localStorage.getItem(prefix + '_' + key);

            } catch (e) {

                return false;

            }

        } else  {

            return false;

        }

    }

    var removeItem = function (key) {

        if (!key) {

            console.error("Missing parameter \"key\"");

            return false;
        }

        if (window.localStorage && window['localStorage']) {

            try {
                return localStorage.removeItem(prefix + '_' + key)
            } catch (e) {
                return false;
            }

        } else  {

            console.log("Browser doen not support HTML5 Web Storage");

        }

    }

    return {
        set: function (key, value) {
            return saveItem(key, value);
        },
        get: function () {
            return getItem(key, item);
        },
        remove: function () {

        }
    }

}

var as = new AppStorage ('MyApp');

Solution

Structure:

Instead of having a function encompass everything, consider using a prototype constructor, or if you can use ECMAScript 2015, use the class structure.

By using the prototype structure your code would look something like the following:

var AppStorage = function(name){
    this.prefix = name;
}
AppStorage.prototype.saveItem = function(key, value){

}
AppStorage.prototype.getItem = function(key){

}
AppStorage.prototype.removeItem = function(key){

}
var AS = new AppStorage('myName');
AS.getItem('thing');


Which removes the need for blocks like the following:

return {
    set: function (key, value) {
        return saveItem(key, value);
    },
    get: function () {
        return getItem(key, item);
    },
    remove: function () {

    }


Things to improve upon:

  • console.error: instead of that, use throw new Error() instead.



  • There's a consistent use of extraneous empty lines, remove them. They're pointless and waste space.



  • When you return the aliases for the functions, the remove function is left empty, if that is the desired case, then it should simply be removed, otherwise, you forgot to link the remove function.



  • window.localStorage && window['localStorage']: uh, that's identical; you can access object properties via the . method, or [''] method.



  • prefix + '_' + key: it would be easier/better to just append the underscore when you define the prefix initially.



  • removeItem is the only function that doesn't just return false if localStorage does not exist. Consider incorporating the error message in every function, or just check for localStorage on initialisation.



With those changes in mind...

... here's your updated code:

var AppStorage = function(name) {
    if (!(this instanceof AppStorage)) {
        throw new Error("AppStorage must be invoked with new!");
    }
    if (!window.localStorage){
        throw new Error("Browser does not support HTML5 Web Storage");
    }
    this.prefix = name + '_';
}
AppStorage.prototype.save = function(key, value) {
    if (!key || !value) {
        throw new Error("Missing parameters \"key\" and \"value\"");
    }
    try {
        localStorage.setItem(this.prefix + key, JSON.stringify(value));
    } catch (e) {
        return false;
    }
};
AppStorage.prototype.get = function(key) {
    if (!key) {
        throw new Error("Missing parameter \"key\"");
    }
    try {
        localStorage.getItem(this.prefix + key);
    } catch (e) {
        return false;
    }
};

AppStorage.prototype.remove = function(key) {
    if (!key) {
        throw new Error("Missing parameter \"key\"");
    }
    try {
        return localStorage.removeItem(this.prefix + key);
    } catch (e) {
        return false;
    }
};
var as = new AppStorage('MyApp');

Code Snippets

var AppStorage = function(name){
    this.prefix = name;
}
AppStorage.prototype.saveItem = function(key, value){

}
AppStorage.prototype.getItem = function(key){

}
AppStorage.prototype.removeItem = function(key){

}
var AS = new AppStorage('myName');
AS.getItem('thing');
return {
    set: function (key, value) {
        return saveItem(key, value);
    },
    get: function () {
        return getItem(key, item);
    },
    remove: function () {

    }
var AppStorage = function(name) {
    if (!(this instanceof AppStorage)) {
        throw new Error("AppStorage must be invoked with new!");
    }
    if (!window.localStorage){
        throw new Error("Browser does not support HTML5 Web Storage");
    }
    this.prefix = name + '_';
}
AppStorage.prototype.save = function(key, value) {
    if (!key || !value) {
        throw new Error("Missing parameters \"key\" and \"value\"");
    }
    try {
        localStorage.setItem(this.prefix + key, JSON.stringify(value));
    } catch (e) {
        return false;
    }
};
AppStorage.prototype.get = function(key) {
    if (!key) {
        throw new Error("Missing parameter \"key\"");
    }
    try {
        localStorage.getItem(this.prefix + key);
    } catch (e) {
        return false;
    }
};

AppStorage.prototype.remove = function(key) {
    if (!key) {
        throw new Error("Missing parameter \"key\"");
    }
    try {
        return localStorage.removeItem(this.prefix + key);
    } catch (e) {
        return false;
    }
};
var as = new AppStorage('MyApp');

Context

StackExchange Code Review Q#108981, answer score: 7

Revisions (0)

No revisions yet.