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

Application that creates a toolbar to show/hide elements on a page

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

Problem

I've been spending a lot of time reading about JavaScript lately, and this is my first piece code that I've tried to apply some of what I've read to. I'm trying to stay clear of using a framework for this so I can have a better understanding of the JavaScript language itself.

This code is part of a chrome extention I wrote to add a toolbar on the bottom of a users GitHub dashboard so they can filter out items.

I'd like to get some feedback on its correctness, if there any best practices I've missed, and if there are any areas where I can be more "functional". Obviously I'd like to know if there are any obvious errors as well.

```
var filterObj = (function(){
var filterObj,
newsItems = [],
hiddenClasses = [],
moreLink,
visibleCount = 30,
filterObjects = [],
filters,
versionKey = "githubNewsFilterVersion",
filterKey = "filters";

//private

var getFilters = function(){
//check for filters in the local storage, otherwise create a new object
if(!localStorage[filterKey]){
filters = {
issueComment : {text: "Issue Comment",id: "issues_comment",checked: false},
pullRequest : {text: "Pull Request & Issue Opened",id: "issues_opened",checked: false},
follow : {text: "Follow",id: "follow",checked: false},
gist : {text: "Gist",id: "gist",checked: false},
push : {text: "Push",id: "push",checked: false},
created : {text: "Created Branch", id:"create",checked: false},
issueClosed : {text: "Close & Merge", id:"issues_closed",checked: false},
fork: {text: "Forked", id: "fork",checked: false},
watch: {text: "Watch", id: "watch_started",checked: false},
editWiki : {text: "Wiki", id: "gollum",checked: false}
};

localStorage[filterKey] = JSON.stringify(filters);
}
else{

Solution

Personally I wouldn't use the pattern

var functionName = function (params, etc) { 
};


as this will make any debugging a PITA. The call stack will show something like:

Anonymous method: line 12
Anonymous method: line 4


etc etc.

Since you already have it inside just do:

(function(){
    function functionName ( params, etc){
    }
})());


and you should be right to go. http://jsfiddle.net/6V26R/

As a personal preference I wouldn't do:

var filterObj = (function(){
    var filterObj,
        //etc


as I find our brains cannot read that so well. Instead I would do:

var filterObj = (function(){
    var filterObjElement,
        //etc


now my brain distinguishes between the global object and the local variable holding an element.

Code Snippets

var functionName = function (params, etc) { 
};
Anonymous method: line 12
Anonymous method: line 4
(function(){
    function functionName ( params, etc){
    }
})());
var filterObj = (function(){
    var filterObj,
        //etc
var filterObj = (function(){
    var filterObjElement,
        //etc

Context

StackExchange Code Review Q#5068, answer score: 7

Revisions (0)

No revisions yet.