patternjavascriptMinor
User model with BackboneJS and RequireJS with test cases in Jasmine
Viewed 0 times
jasminewithuserrequirejsandtestbackbonejsmodelcases
Problem
I've been taking a stab at implementing test cases using the Jasmine testing framework. To do so, I've made an application which has a User object. This User object is created by the server, but its ID is stored in a couple of client-side data storage locations. When a new User model is initialized I check those storage locations before fetching User data from the server.
My model is doing too much work in the constructor. This is making it difficult to test. I am wondering how I could improve my architecture to make it easier to test and also if there are any other mistakes with how I am implementing Backbone, Require, and Jasmine.
```
// A singleton representing the sole logged on user for the program.
// Tries to load itself by ID stored in localStorage and then by chrome.storage.sync.
// If still unloaded, tells the server to create a new user and assumes that identiy.
define(['programState'], function(programState) {
'use strict';
var userIdKey = 'UserId';
// Loads user data by ID from the server, writes the ID
// to client-side storage locations for future loading and then announces
// that the user has been loaded fully.
function fetchUser(shouldSetSyncStorage) {
this.fetch({
success: function(model) {
// TODO: Error handling for writing to sync too much.
// Write to sync as little as possible because it has restricted read/write limits per hour.
if (shouldSetSyncStorage) {
chrome.storage.sync.set({ userIdKey: model.get('id') });
}
localStorage.setItem(userIdKey, model.get('id'));
// Announce that user has loaded so managers can use it to fetch data.
model.trigger('loaded');
},
error: function (error) {
console.error(error);
}
});
}
// User data will be loaded either from cache or server.
var User =
My model is doing too much work in the constructor. This is making it difficult to test. I am wondering how I could improve my architecture to make it easier to test and also if there are any other mistakes with how I am implementing Backbone, Require, and Jasmine.
```
// A singleton representing the sole logged on user for the program.
// Tries to load itself by ID stored in localStorage and then by chrome.storage.sync.
// If still unloaded, tells the server to create a new user and assumes that identiy.
define(['programState'], function(programState) {
'use strict';
var userIdKey = 'UserId';
// Loads user data by ID from the server, writes the ID
// to client-side storage locations for future loading and then announces
// that the user has been loaded fully.
function fetchUser(shouldSetSyncStorage) {
this.fetch({
success: function(model) {
// TODO: Error handling for writing to sync too much.
// Write to sync as little as possible because it has restricted read/write limits per hour.
if (shouldSetSyncStorage) {
chrome.storage.sync.set({ userIdKey: model.get('id') });
}
localStorage.setItem(userIdKey, model.get('id'));
// Announce that user has loaded so managers can use it to fetch data.
model.trigger('loaded');
},
error: function (error) {
console.error(error);
}
});
}
// User data will be loaded either from cache or server.
var User =
Solution
From a once over:
I don't think that too much is going in
- The trigger name
'loaded'should be a constant, declared on theuserIdKeylevel
- You should not be using
console.log, at the very least you should consoledate those calls into a function.
- It seems that if the user needs to be created thru
.save(), that you are not storing theuserIdKeyneither inlocalStoragenorchrome.storage, rendering most of the code meaningless? I could be wrong.
- It also seems that if you find the
localStorageitem, you force the writing tochrome.storage, why ? If it's in thelocalStorage, I would expect it in thechrome.storageas well, at the very least you should try agetbefore you try asetto reduce writing tochrome.storage?
fetchUsershould be part of the model, then you dont have to use.call()
- JsHint could find nothing to complain about
- Commenting might be a tad excessive
- I can easily understand what the code does (that is, if my prior analysis is correct)
I don't think that too much is going in
initialize, but I do think that you need to work some more on this code.Context
StackExchange Code Review Q#20814, answer score: 2
Revisions (0)
No revisions yet.