patternjavascriptMinor
Can this localStorage login script be written more efficiently?
Viewed 0 times
thisscriptcanefficientlywrittenloginmorelocalstorage
Problem
I understand that account details should be stored using a much more secure method, but this is only a demonstration script I've made to store login credentials and remember a user wishes to remain logged in using
Could anyone show me how I might write this more efficiently please? My main concern is code repetition, so I'd love to move toward a much more object oriented structure.
```
var bic = bic || {};
bic.checkLogin = function() {
var loginUser = document.getElementById("loginUser").value;
var loginPass = document.getElementById("loginPass").value;
var storedUser = localStorage.Username;
var storedUser2 = storedUser.replace(/"/g,''); // remove " for plain text display in loginForm
var storedPass = localStorage.Password;
var checked = document.getElementById("rememberLogin").checked;
if(checked == true) {
localStorage.setItem("loggedIn", "yes");
} else {
localStorage.setItem("loggedIn", "no");
}
// if Username key exists, and user/pass values match storage entries
if(localStorage.Username) {
if(storedUser.match(loginUser) && storedPass.match(loginPass)) {
bic.notify("loginForm", "Logged in as: " + storedUser2 + "Log out?");
document.getElementById("logout").onclick = function(e){e.preventDefault();bic.logout();};
console.log("Logged in as: " + localStorage.Username);
} else {
bic.loginFailed();
bic.notify("loginStatus", "Login failed. Please try again.");
console.log("Failed logins: " + localStorage.failedLogins);
}
}
}
// localStorage equiv of cookie
bic.isLoggedIn = function() {
var a = localStorage.loggedIn;
var storedUser = localStorage.Username;
var storedUser2 = storedUser.replace(/"/g,''); // removing " as above
// does a exactly equal "yes"?
if(a === "yes") {
bic.notify("loginForm", "Logged in as: " + storedUser2 + "Lo
localStorage rather than sessionStorage.Could anyone show me how I might write this more efficiently please? My main concern is code repetition, so I'd love to move toward a much more object oriented structure.
```
var bic = bic || {};
bic.checkLogin = function() {
var loginUser = document.getElementById("loginUser").value;
var loginPass = document.getElementById("loginPass").value;
var storedUser = localStorage.Username;
var storedUser2 = storedUser.replace(/"/g,''); // remove " for plain text display in loginForm
var storedPass = localStorage.Password;
var checked = document.getElementById("rememberLogin").checked;
if(checked == true) {
localStorage.setItem("loggedIn", "yes");
} else {
localStorage.setItem("loggedIn", "no");
}
// if Username key exists, and user/pass values match storage entries
if(localStorage.Username) {
if(storedUser.match(loginUser) && storedPass.match(loginPass)) {
bic.notify("loginForm", "Logged in as: " + storedUser2 + "Log out?");
document.getElementById("logout").onclick = function(e){e.preventDefault();bic.logout();};
console.log("Logged in as: " + localStorage.Username);
} else {
bic.loginFailed();
bic.notify("loginStatus", "Login failed. Please try again.");
console.log("Failed logins: " + localStorage.failedLogins);
}
}
}
// localStorage equiv of cookie
bic.isLoggedIn = function() {
var a = localStorage.loggedIn;
var storedUser = localStorage.Username;
var storedUser2 = storedUser.replace(/"/g,''); // removing " as above
// does a exactly equal "yes"?
if(a === "yes") {
bic.notify("loginForm", "Logged in as: " + storedUser2 + "Lo
Solution
Separate UI from logic
You might want to keep the logic for localStorage handling separate from the DOM fetching logic. That way, your system for handling the local storage can easily be detached and reused elsewhere without that extra baggage. Also works the other way around, when you replace your UI.
Basic login API
In continuation with the previous paragraph, you could model it to have this API:
The UI logic is separate. It consumes this API rather than being one with it.
LocalStorage is synchronous
Note that localStorage is a synchronous operation. Doing heavy IO against localStorage could mean an unresponsive experience. This has an amplified effect on mobile devices, since they are times slower. Don't pull out or put in huge data. Do it by chunks. Use multiple small keys rather than one big key to house everything your system needs.
JSON.parse() and JSON.stringify()
localStorage is a string-based, key-value storage. But that's it. You need some form of format to structure highly complex data, like user data. Something that may look like:
To store data like a JS object, you need
Storage routine
What I typically do when using localStorage is to create an in-memory object of the data. On every save, I serialize the data with
Code refinements
Since I advertised array-based storage, then Array filter, splice, push would be your best friends here. In addition, I believe I introduced you to JSON stringify and parse already.
You might want to keep the logic for localStorage handling separate from the DOM fetching logic. That way, your system for handling the local storage can easily be detached and reused elsewhere without that extra baggage. Also works the other way around, when you replace your UI.
Basic login API
In continuation with the previous paragraph, you could model it to have this API:
bic.login(username,password); // Return true if logged in, false if not
bic.isLoggedIn(); // Return true if someone is logged in, else false
bic.getUserId(); // Gets the current user's ID
bic.logout(); // Logout current user
bic.register(username,password) // Registration
bic.destroyCurrentUser(); // Just for fun. Only a logged in user can delete himselfThe UI logic is separate. It consumes this API rather than being one with it.
LocalStorage is synchronous
Note that localStorage is a synchronous operation. Doing heavy IO against localStorage could mean an unresponsive experience. This has an amplified effect on mobile devices, since they are times slower. Don't pull out or put in huge data. Do it by chunks. Use multiple small keys rather than one big key to house everything your system needs.
JSON.parse() and JSON.stringify()
localStorage is a string-based, key-value storage. But that's it. You need some form of format to structure highly complex data, like user data. Something that may look like:
[
{id:1,name:"foo",password:"***"},
{id:1,name:"bar",password:"***"},
{id:1,name:"baz",password:"***"}
]To store data like a JS object, you need
JSON.stringify() to turn it into a string. Then you can store it to localStorage. To get it, retrieve it from localStorage and then use JSON.parse() to convert from string.Storage routine
What I typically do when using localStorage is to create an in-memory object of the data. On every save, I serialize the data with
JSON.stringify and store it to the localStorage. If ever there are other code that modifies that same data, there are localStorage events to tell you if the data is modified in the storage.Code refinements
Since I advertised array-based storage, then Array filter, splice, push would be your best friends here. In addition, I believe I introduced you to JSON stringify and parse already.
Code Snippets
bic.login(username,password); // Return true if logged in, false if not
bic.isLoggedIn(); // Return true if someone is logged in, else false
bic.getUserId(); // Gets the current user's ID
bic.logout(); // Logout current user
bic.register(username,password) // Registration
bic.destroyCurrentUser(); // Just for fun. Only a logged in user can delete himself[
{id:1,name:"foo",password:"***"},
{id:1,name:"bar",password:"***"},
{id:1,name:"baz",password:"***"}
]Context
StackExchange Code Review Q#48098, answer score: 6
Revisions (0)
No revisions yet.