Приложение, которое создает панель инструментов показать/скрыть элементы на странице


Я провожу много времени в последнее время читать о JavaScript, и это мой первый кусок кода, который я пытался применить некоторые из того, что я читал. Я пытаюсь держаться подальше от использования платформы для этого, так я могу лучше понять сам язык JavaScript.

Этот код является частью расширение для Chrome я написал, чтобы добавить панель инструментов в нижней части с GitHub для пользователей, чтобы они могли отфильтровать элементы.

Я хотел бы получить некоторую обратную связь на его правильность, если есть какие-либо лучшие практики, которые я упустил, И если есть какие-то области, где я могу быть более "функциональный". Очевидно, я хотел бы знать, если есть какие-то очевидные ошибки, а также.

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{
            filters = JSON.parse(localStorage[filterKey]);
        }
    };

    var getNewsItems = function(callback){
        var items = getElementsByClass("div","alert"),
            len = items.length,
            newsLength = newsItems.length,
            found = false,
            currentItem = "";

        for(var i = 0; i < len; i++){

            found = false;
            currentItem = items[i];
            //check that the items isn't in the list
            for(var x = 0; x < newsLength; x++) {
                if(newsItems[x] == currentItem){
                    found = true;
                    break;
                }
            }

            if(!found){
                newsItems.push(items[i]);
            }
        }

        if(callback){
            callback();
        }
    };

    var createDiv =  function(){
            var newDiv = document.createElement("div");
            newDiv.id = "filterDiv";
            newDiv.className = "filterBar";

            document.getElementById("footer").appendChild(newDiv);

            filterObj = newDiv;

            createImg();
    };

    var createImg = function(){
        var closeSpan = document.createElement("span"),
            closeImage = document.createElement("img");

        closeImage.src = chrome.extension.getURL("assets/close.png");
        closeSpan.className = "closeBtn";

        closeImage.addEventListener("click",function(){
            document.getElementById("filterDiv").style.display = "none";    
        });

        closeSpan.appendChild(closeImage);
        filterObj.appendChild(closeSpan);
    };

    var createElement = function(theType, theID, theName, theValue, theAttrs, theClass){
        var newElem = document.createElement(theType),
            prop; 

        newElem.id = theID || "";
        newElem.name = theName || "";
        newElem.value = theValue || "";
        newElem.className = theClass || "";

        for(prop in theAttrs){
            if(theAttrs.hasOwnProperty(prop)){
                newElem[prop] = theAttrs[prop];
            }
        }

        return newElem;
    };

    var setFilters = function(){
        var prop; 

        for(prop in filters){

            var newFilterOption = createElement("input",
                                                prop,
                                                prop,
                                                filters[prop].id,
                                                {type : "checkbox"});
            addListener(newFilterOption);

            if(filters[prop].checked){
                newFilterOption.checked = true;
            }

            var newFilterLabel = createElement("label");
            newFilterLabel.innerHTML = filters[prop].text;  

            var newFilterWrapper = createElement("span");

            newFilterWrapper.className = "filterOption";
            newFilterWrapper.appendChild(newFilterLabel);
            newFilterWrapper.appendChild(newFilterOption);

            filterObjects.push(newFilterOption);

            filterObj.appendChild(newFilterWrapper);
        }
    };

    var addListener = function(elem){
        elem.addEventListener("change",function(){

            var newsObjects = newsItems,
                len = newsObjects.length,
                i;

            if(elem.checked === true){
                //loop through the elements array instead
                for(i = 0; i < len; i++){
                    if(hasClass(newsObjects[i], elem.value)){
                        newsObjects[i].style.display = "none";  
                        --visibleCount;
                    }
                }
            }
            else{
                for(i = 0; i < len; i++){
                    if(hasClass(newsObjects[i], elem.value)){
                        newsObjects[i].style.display = "inherit";   
                        ++visibleCount;
                    }
                }   
            }

            setFilterVal(elem.value,elem.checked);
        }); 
    };

    var setFilterVal = function(className,isChecked){

        for(filterObj in filters){

            if(filters[filterObj].id === className){
                filters[filterObj]["checked"] = isChecked;
                break;
            }
        }

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

    var getMoreLink = function(){
        var moreDiv = getElementsByClass('div',"ajax_paginate")[0];
        moreLink = moreDiv.firstChild;

        attachClickListener();
    };

    var attachClickListener = function(){
        moreLink.addEventListener("click",function(){
                var i = 0;                          

                var intervalID = window.setInterval(function(){
                    i++;

                    getNewsItems(runFilters);

                    if( i === 20 ){
                        //reattach the event
                        getMoreLink();
                        window.clearInterval(intervalID);
                    }

                },200);
        });
    }

    var runFilters = function(){
        var len = filterObjects.length;

        for(var i = 0; i < len; i++){

            if(filterObjects[i].checked === true){

                var evt = document.createEvent("HTMLEvents");
                evt.initEvent("change", false, true);
                filterObjects[i].dispatchEvent(evt);
            }
        }

    }

    //looks like this function and the function below it can be rolled up into a partial
    //application
    var getElementsByClass = function(startTag, theClass){

        //if no start tag was specified then get all the elements
        var elements = startTag ? document.getElementsByTagName(startTag) : document.all;

        var matches = [];
        var pattern = new RegExp("(^| )" + theClass + "( |$)");

        for(var i =0; i< elements.length; i++){
            if(pattern.test(elements[i].className)){
                matches.push(elements[i]);
            }
        }

        return matches;
    };

    var hasClass = function(elem, theClass){

        var pattern = new RegExp("(^| )" + theClass + "( |$)"); 
        return pattern.test(elem.className);
    };

    var checkVersion = function(){
        var storedVersion = localStorage[versionKey],
            currentVersion = manifest.version;

        if(!storedVersion || storedVersion !== currentVersion){

            localStorage.removeItem(filterKey);
            localStorage[versionKey] = currentVersion;


        }
    };

    var manifest = (function() {
        var manifestObject = false;
        var xhr = new XMLHttpRequest();

        xhr.onreadystatechange = function() {
            if (xhr.readyState == 4) {
                manifestObject = JSON.parse(xhr.responseText);
            }
        };
        xhr.open("GET", chrome.extension.getURL('/manifest.json'), false);

        try {
            xhr.send();
        } catch(e) {

        }
        return manifestObject;
    })();

    //public
    return {
        Init : function(){
            checkVersion();
            getFilters();
            createDiv();
            getNewsItems();
            setFilters();
            getMoreLink();
            runFilters();
        }
    };  
}());

filterObj.Init();


398
11
задан 29 сентября 2011 в 07:09 Источник Поделиться
Комментарии
4 ответа

Я собираюсь отправить некоторые из отзывов, которые я получил от группы jsmentors на Google. Это такие вещи, которые я ищу.

От Нейтана Сладкий:



  1. Существует структурная проблема в том, как ты организуешь ты код.
    Вы выставляете две глобальные переменные, когда вам не нужно, не то что
    вопросам, что многое в этом случае. Вы используете простой
    выполнить--код-очередной сценарий, который я лично думаю, что гарантирует использование
    анонимный объект, но анонимная функция работает хорошо. Все-таки
    нет необходимости инициализировать свой код за пределами функции, в
    ведь если все было в объявлении функции можно инициализировать
    выше всех своих функций. Это то, что анонимный объект выглядит
    как: ({
    Инит :функция(){/запустить код/},
    метод1:функция(){это.метода Method2();},
    метода Method2:функция(){/делает что-то/} }).Инит()

  2. Просто придирчивые, вы создали метод getElementsByClass, и
    Я не совсем уверен, что он делает, но я вижу, вы проверяете
    отдельного класса элементов в методе. Хром имеет
    встроенный способ получения элементов по классам.

  3. Линии 66-81 болезненны, и я думаю вы это знаете. Вместо
    цикл через каждый элемент, попробуйте дать все элементы уникальной
    ID, или что-то подобное, так что вы можете хэш-результаты одного
    из этих массивов и создать О(Н) петля вместо О(Н*Н) образец.


Из Остина Чейни:


Это назначение способ создания функции: ВАР все =
функция () {};

Это заявление путем создания функции: функция что
() {}

Второй подсадил, который часто удобно и редко
проблема. Однако, в тех экзотических случаях, когда грузоподъемная является
проблема это катастрофическая. Нет ничего вы можете сделать, чтобы предотвратить
ключевое слово var быть поднят, но вы можете помешать функций
будучи подсадили. Единственное исключение о подъемных и объявил
функции-это когда функция автоматически вызывается, как и
функция "filterObj" в коде. Это происходит потому, что функция
выполняет сразу в своем нынешнем виде и положении.

Общим аргументом в пользу объявлении метода, который я
верю Роб предполагал, что для некоторых людей декларации
способ легче читать, потому что вы можете быстро определить, что или
не является функцией. Это, безусловно, актуальны, и я даже соглашусь с
это в полной мере для небольших приложений. Для больших приложений
содержащий множество функций этот аргумент терпит неудачу. Какое значение в
пытаясь определить, что является или не является функцией, если у вас есть
контейнер с 60 функций? В этот момент он значительно быстрее
чтобы прочитать код, ищет идентификатор, а затем определить
является ли идентификатор является или не является функцией, прочитав очень
следующего слова справа.

Я также считаю, что декларативный способ является более сложным для поддержания
исключительно большое приложение, потому что он не привязан к Варе
ключевое слово. Я считаю, что привязка ссылок, будь то переменные или
имена функций, для одного ключевого слова var резко снижает
сложность на самом деле читать код. Из-за этого я всегда
рекомендуем использовать не более одного ключевого слова var в функции и обеспечение
что ничто не возникает до этого ключевого слова var, за исключением немедленно
вызываются функции и "использовать строгие" ПРАГМА. Этой одной переменной правило
при использовании "использовать строгие" ПРАГМА результаты в целом более
прочный и портативный код, а также код, который легче читать.

Я вижу, что вокруг линии 298 вы не используя try/catch блок. Я
омерзителен попробовать/catch блоки, как дешевая попытка смягчить
известные ошибки. Если вы не осведомлены о возможности ошибки потом исправить
ваш код. Это ошибка является результатом ввода данных пользователем после вывода
ответ на этот эффект, так что пользователь будет знать, почему казнили
не удалось выполнить, как ожидалось.

Я тоже не вижу смысла в использовании прибыли на анонимный объект
буквальном В конце вашей заявки. Очевидно, что это идет на
архитектура приложения в том, что вы хотите серию
суб-глобальные функции для выполнения в определенном порядке, которые могут
ничего не вернуть, но тем не менее результат в глобальном
"filterObj" возвращение некоторым объектным литералом. Это старая конвенции,
в частности, использование чего-то под названием "инит", что строки вместе
серия не связанных между собой смертную казнь. Часть причины, вы, вероятно,
с помощью этой Конвенции заключается в том, что все функции в вашем приложении
по-видимому, существуют в равном объеме непосредственно в рамках глобальной
"filterObj" содержать, а это неэффективно. Только создать функции
в области, где они необходимы, или если некоторые функции необходимы
разных местах, то при минимально возможном объеме для повторного использования.
Это позволяет уменьшить поисков, что существенно увеличивает исполнения
скорость. Скорость в JavaScript сводится к уменьшению поисков и
используя наиболее подходящего оператора или метода для данной работы.

У вас также есть некоторые незначительные нарушения синтаксиса в коде. Например
в "attachClickListener" отсутствует завершающий запятой. Это
бы не допустить ошибок минификации кода. Я бы
предложение о применении упомянутых выше рекомендаций и затем после
отправка кода с помощью JSLint.

От Федорова "БГА" Александр:


1) преобразовать ваш код

 {    const singletonObject = (function(){ 
// helper fns
return {
// privare members
newsItems_: [],
_init: function(){
// your object init here
delete this._init // prevent double init
return this
},
// methods
getFilters: function(){
}
}._init() })() }


2) Замените {!localStogare[ФОО]} до {localStogare[фу] != значение null}
3) поставить 1 дополнительный пробел после запятой в коде ivoke ФН { _fn(А, B) }
4) у вас есть монстр ФН {метод createElement}. Слишком много аргументов. Плз использовать объект конф. {метод createElement('див', {идентификатор: 'фу'})}
5) Вы можете забыть о запятой
6) попробуйте использовать стиль ФН в JS максимально, не

{getElementsByClass('div', 'foo')},    { 
el.getElementsByTag('div')._map(function(v){ return
v.getElementsByClass('foo') })._reduce(function(v1, v2){ return
v1.concat(v2) }) }


7) Используйте {localStogare.метод getitem}, а не {localStogare[]}
8) распределение Варс в месте, где вы его используете. Например> в {getNewsItems} вы к alloc {нашли} в верхней части, но должны в 68 линии
9) вы используете прямой здании Дом с помощью {документ.метод createElement}, использовать > шаблоны, его более читабельным и ремонтопригодны, чем набор {метода appendChild} и {метод createElement} )
10) в строках 71 - 75 у вас > {массив.прототип.метод indexOf}
11) хорошо, что вы используете Правило 1 ВАР = 1 строка

Полный поток может быть найден здесь.

3
ответ дан 5 октября 2011 в 08:10 Источник Поделиться

Лично я бы не использовать шаблон

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

как это сделает любой отладку лаваш. Стек вызовов покажет что-то вроде:

Anonymous method: line 12
Anonymous method: line 4

и т. д. и т. п.

Поскольку вы уже имеете его внутри просто делать:

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

и вы должны быть право уйти. http://jsfiddle.net/6V26R/

В качестве личного предпочтения, я бы не стал делать:

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

как я считаю, наш мозг не может прочитать, что так хорошо. Вместо этого я хотел бы сделать:

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

теперь мой мозг проводит различие между глобальным объектом и локальной переменной, содержащей элемент.

7
ответ дан 4 октября 2011 в 01:10 Источник Поделиться

С точки зрения практики, больше комментариев будет идти долгий путь. Также использование последовательной верблюжьего хорошо и ценят. Одна хорошая практика является довольно простым в реализации является объявление всех переменных в начале область / функции.

5
ответ дан 3 октября 2011 в 04:10 Источник Поделиться

Почему бы не продолжить свой первоначальный запятую ВАР декларации по своей функции? Вот так:

versionKey = "githubNewsFilterVersion",
filterKey = "filters",
getFilters = function(){
...
},
getNewsItems = function(callback){
...

И так далее.

0
ответ дан 4 октября 2011 в 08:10 Источник Поделиться