6
\$\begingroup\$

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');
\$\endgroup\$
7
\$\begingroup\$

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');
\$\endgroup\$
  • \$\begingroup\$ Thank you so much for the answer @Quill. I have one more question. With this pattern how can i set local methods and variables which are only available with in the constructor. for an example i want prefix variable to be only available locally but it should be able to refer with in the prototype methods \$\endgroup\$ – user3405435 Oct 28 '15 at 12:18
  • 1
    \$\begingroup\$ @user3405435: If that's really a requirement, then you'll need to stick with your original pattern, effectively making your methods into closures that carry a reference to the "private" prefix variable. Outside of scoping tricks like that, there's no way (AFAIK) to have actual "private" properties on JS objects. That said, if you just want to prevent other code from (easily) changing the prefix, making it read-only might be enough. \$\endgroup\$ – Ilmari Karonen Oct 28 '15 at 13:05
  • \$\begingroup\$ @user3405435, actually that's where the this keyword comes into play. In any prototype (including the constructor) you can define variables to be used throughout any method attached to the prototype. In my example, in the functions save, get and return: prefix was changed to this.prefix. \$\endgroup\$ – Quill Oct 28 '15 at 13:11
  • \$\begingroup\$ @vivekpoddar: if you see my final example, they're left out. \$\endgroup\$ – Quill Oct 29 '15 at 5:10
  • \$\begingroup\$ Ah, right. Forgot about those, thanks for the reminder. \$\endgroup\$ – Quill Nov 3 '15 at 21:50

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.