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

Status display module

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
displaymodulestatus

Problem

I've developed a template to modularize JavaScript code I write for client work. The example below is a simple class with 1 attribute and 3 functions.

Rationale:

  • Standardized constructor to add or modify settings/ options



  • Functions as variables used later to return only "public" functions, all else are private



  • Using local "call" variable to easily replace functions in unit testing



Are there any improvements you can recommend or general best practices to look for in terms of improvement?

```
var dataStatus = function( constructorOptions )
{
"use strict";

var options = {
displayId: 'dataStatus',
statii: {
'settingUp': {
tooltip: 'Setting Up...',
color: '808080'
},
'loading': {
tooltip: 'Data Loading...',
color: '99ee90'
},
'loaded': {
tooltip: 'Data Loaded',
color: '006400'
},
'changed': {
tooltip: 'Data Changed',
color: '8b0000'
},
'saving': {
tooltip: 'Saving...',
color: 'ffd700'
}
},
defaultStatus: 'settingUp'
};

var local = {
status: false
};

var init = function( optionsToSet )
{
jQuery.extend( options, optionsToSet);

call.setStatus( options.defaultStatus );
return this;
};

var inject = function (functionToReplace, injectedFunction)
{
call[functionToReplace] = injectedFunction;
return injectedFunction;
};

var getStatus = function()
{
return local.status;
};

var setStatus = function( status, signal )
{
local.status = status;
if (typeof signal == 'undefined' || signal != false){
call.signal( call.getStatus() );
}
};

var signal = function ( status ){
if (status in options.

Solution

Your code has somewhat inconsistent styling which can inhibit readability and prevent the code from looking crisp and professional. For example, some of your function definitions have braces on the next line others don't. Some of your function definitions and calls put spaces between parenthesis var signal = function ( status ){, and some don't var inject = function (functionToReplace, injectedFunction).

Also, I don't believe that the last line init( constructorOptions ); is executing because you return from the constructor in both the if and else blocks right above it.

That said, using closures to keep private functions private is a great idea.

As for the modularity, while returning a separate object for for testing is a truly fascinating idea, my gut instinct goes against it. I find it somewhat troublesome because it violates cohesion with your constructor really doing two things: creating a dataStatus object and creating a dataStatus test object. This introduces room for bugs if future functions aren't properly assigned to both the real and test object. To catch these bugs, you'd want to write a test case that ensures the test object shares all the same functions as the real object. (i.e. call could have extra functions but it must also have each one that a normal dataStatus would)

I'd recommend eliminating call and if needed to use this instead. Although it might be best to just call the functions directly.

var init = function () {
    jQuery.extend(options, optionsToSet);

    setStatus(options.defaultStatus); // just call setStatus directly
    return this;
}


You can still do your testing by dissecting the methods you want from the real object.

var realObj = new dataStatus (options);
var testObj = {
    setStatus : realObj.setStatus,
    getStatus : function () {},
    signal : function () {}
};


Private functions should be tested through the testing of public functions, but in your case the only private function is for changing functions when testing.

UPDATE:

If you're set on testing your private functions--and I can fully understand how it could be worth it--you ought to to take care of a few things.

First, make sure the test object and the real object have the same functions. You could write a test case that checks that their functions are the same, but it may be best to just write the private functions directly as properties of the call object:

var call = {};

call.init = function () {
    // ...
}
call.inject = function () {
    // ...
}
call.getStatus = function () {
    // ...
}

// ...

if (/* determine if your unit testing each function */) {
    return call;
}
else {
    return {
        init: call.init,
        getStatus: call.getStatus,
        setStatus: call.setStatus,
        signal: call.signal
    };
}


Secondly, you will want to also test the real object. You could have millions of test cases for your test object, but since your real object isn't your test object, the real object might still break.

Code Snippets

var init = function () {
    jQuery.extend(options, optionsToSet);

    setStatus(options.defaultStatus); // just call setStatus directly
    return this;
}
var realObj = new dataStatus (options);
var testObj = {
    setStatus : realObj.setStatus,
    getStatus : function () {},
    signal : function () {}
};
var call = {};

call.init = function () {
    // ...
}
call.inject = function () {
    // ...
}
call.getStatus = function () {
    // ...
}

// ...

if (/* determine if your unit testing each function */) {
    return call;
}
else {
    return {
        init: call.init,
        getStatus: call.getStatus,
        setStatus: call.setStatus,
        signal: call.signal
    };
}

Context

StackExchange Code Review Q#125970, answer score: 3

Revisions (0)

No revisions yet.