Простые звезды системы


Я работаю на простой звездочной системы, и прежде чем я перейду к собственно "подсчета" голосов, которые я буду делать с $.Аякс, я хотел убедиться, что я сделал до сих пор было сделано так же, как это может быть.

В JavaScript

//max number of stars
var max_stars = 5;

//add images to the ratings div
function fill_ratings() {
    var $rating = $("#rating");
    if($rating.length) {
        for(i = 1; i <= max_stars; i++) {
            $rating.append('<img id="' + i + '"src="star_default.png" />');
            }
        }
    }

//variables...
var hovered = 0;
var has_clicked = false;
var star_clicked = 0;

function handle_ratings() {

    if(has_clicked == true) {
        for(i = 1; i <= star_clicked; i++) {
            $('#rating img#' + i).attr('src', 'star.png');
        }
    }

    $('#rating img').hover(function() {
        hovered = Number($(this).attr('id'));
        for(i = 1; i <= max_stars; i++) {
            if(i <= hovered) {
                $('#rating img#' + i).attr('src', 'star.png');
                } else {
                    $('#rating img#' + i).attr('src', 'star_default.png');
                }
            }
        }).click(function() {
                //if they click a star, only set the stars to grey that are after it.
                star_clicked = Number($(this).attr('id'));
                has_clicked = true;
                    for(i = 1; i <= star_clicked; i++) {
                        $('#rating img#' + i).attr('src', 'star.png');
                        //handle the vote here. eventually.
                    }                               
                }).mouseout(function() {
                    //if they haven't clicked a star, set all stars to grey.
                    if(has_clicked !== true) {
                            $('#rating img').attr('src', 'star_default.png');
                        } else {
                            //show their rating
                            for(i = 1; i <= max_stars; i++) {
                                if(i <= star_clicked) {
                                    $('#rating img#' + i).attr('src', 'star.png');
                                } else {
                                    $('#rating img#' + i).attr('src', 'star_default.png');  
                                }
                            }       
                        }
                    });
                }

$(function() {
    fill_ratings();
    handle_ratings();
    });

HTML-код

<div id="ratings"></div>

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



2647
12
задан 30 января 2011 в 01:01 Источник Поделиться
Комментарии
1 ответ

Очевидное улучшение кода состоит в том, что вы должны упаковать его как плагин jQuery, чтобы улучшить повторное использование кода и уменьшить количество создаваемых глобальных переменных.

Вы также хотите использовать класс вместо идентификатора, чтобы улучшить возможность повторного использования (если вы решили не менять это плагин), и хранить как можно больше информации о элемента себя, вместо того, чтобы использовать глобальные переменные для хранения информации, для создания менее в сочетании функций.

В противном случае, идя по код построчно:

Внутри fill_ratings функции:

$rating.append('<img id="' + i + '"src="star_default.png" />');

Вы создаете элементы с идентификатором атрибута, начиная с цифры. Это незаконно до того, как HTML5. Кроме того, вы можете создать элементы в более 'библиотека jQuery' образом этот синтаксис вместо:

$('<img>', {
id: 'star-' + i,
src: 'star_default.png'
}).appendTo($rating);

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

Объявление переменной - в основном в стиле предположение, но вы можете избежать несколько ВАР- ов с помощью запятыми:

var hovered = 0, 
has_clicked = false,
star_clicked = 0;

Не использовать $(это) , чтобы получить элемент атрибутами:

$(this).attr('id');

около 97% медленнее, чем

this.id;

последний более компактен и гораздо более эффективным.

Внутри каждого из обработчиков событий, прикрепленные к #оценкой ИМГ - вместо перебора всех #оценкой в img элемент, как вы делаете:

    for (i = 1; i <= max_stars; i++) {
if (i <= hovered) {
$('#rating img#' + i).attr('src', 'star.png');
} else {
$('#rating img#' + i).attr('src', 'star_default.png');
}
}

Мы всегда можем использовать уже дали информацию (текущий элемент, который вызывает событие) для обработки. Приведенный выше код, например, может быть записан как:

$(this).prevAll().attr('src', 'star.png');

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

Типа принуждения к количество может быть сделано без использования ряда - просто добавить + в передней части переменных. В вашем случае, это совершенно лишнее, так как ты принуждающее его обратно в строку снова после присоединения ее к другой строке в селектор прошел в jQuery селектор

14
ответ дан 30 января 2011 в 03:01 Источник Поделиться