patternjavascriptMinor
Application that creates a toolbar to show/hide elements on a page
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{
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
as this will make any debugging a PITA. The call stack will show something like:
etc etc.
Since you already have it inside just do:
and you should be right to go. http://jsfiddle.net/6V26R/
As a personal preference I wouldn't do:
as I find our brains cannot read that so well. Instead I would do:
now my brain distinguishes between the global object and the local variable holding an element.
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 4etc 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,
//etcas I find our brains cannot read that so well. Instead I would do:
var filterObj = (function(){
var filterObjElement,
//etcnow 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,
//etcvar filterObj = (function(){
var filterObjElement,
//etcContext
StackExchange Code Review Q#5068, answer score: 7
Revisions (0)
No revisions yet.