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

User model with BackboneJS and RequireJS with test cases in Jasmine

Submitted by: @import:stackexchange-codereview··
0
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 =

Solution

From a once over:

  • The trigger name 'loaded' should be a constant, declared on the userIdKey level



  • 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 the userIdKey neither in localStorage nor chrome.storage, rendering most of the code meaningless? I could be wrong.



  • It also seems that if you find the localStorage item, you force the writing to chrome.storage, why ? If it's in the localStorage, I would expect it in the chrome.storage as well, at the very least you should try a get before you try a set to reduce writing to chrome.storage ?



  • fetchUser should 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.