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');
1 Answer 1
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, usethrow 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, theremove
function is left empty, if that is the desired case, then it should simply be removed, otherwise, you forgot to link theremove
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 justreturn false
iflocalStorage
does not exist. Consider incorporating the error message in every function, or just check forlocalStorage
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');
-
\$\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– user34054352015年10月28日 12:18:22 +00:00Commented Oct 28, 2015 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– Ilmari Karonen2015年10月28日 13:05:15 +00:00Commented Oct 28, 2015 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 functionssave
,get
andreturn
:prefix
was changed tothis.prefix
. \$\endgroup\$Quill– Quill2015年10月28日 13:11:52 +00:00Commented Oct 28, 2015 at 13:11 -
\$\begingroup\$ @vivekpoddar: if you see my final example, they're left out. \$\endgroup\$Quill– Quill2015年10月29日 05:10:27 +00:00Commented Oct 29, 2015 at 5:10
-
\$\begingroup\$ Ah, right. Forgot about those, thanks for the reminder. \$\endgroup\$Quill– Quill2015年11月03日 21:50:38 +00:00Commented Nov 3, 2015 at 21:50
Explore related questions
See similar questions with these tags.