Einfaches Sterne-Bewertungssystem

2689
Andrew

Ich arbeite an einem einfachen Sterne-Bewertungssystem und bevor ich mich dazu bewege, die Stimmen tatsächlich zu "zählen", womit ich denke $.ajax, dass ich es einfach machen werde, wollte ich sicherstellen, dass das, was ich bisher gemacht habe, als getan wurde gut wie es sein kann.

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>

Der Code funktioniert, aber ich habe mich gefragt, ob jemand Code bemerkt, den ich verbessern kann. Stimmt auch etwas mit der Art und Weise, wie es aussieht? Soll ich mehr kommentieren, anders einrücken oder irgendwelche anderen "Formatierungs" -Dinge?

Antworten
12
Die Verwendung von Namespaces für Ihre globalen Variablen sowie für Methoden ist eine bewährte Methode. vor 9 Jahren 0

2 Antworten auf die Frage

14
Yi Jiang

Die offensichtliche Verbesserung des hier zur Verfügung stehenden Codes besteht darin, dass Sie ihn als jQuery-Plugin packen sollten, um die Wiederverwendbarkeit von Code zu verbessern und die Anzahl der erstellten globalen Variablen zu reduzieren.

Sie sollten auch a classanstelle von verwenden id, um die Wiederverwendbarkeit zu verbessern (wenn Sie dies nicht in ein Plugin ändern möchten ) und so viele Informationen auf dem Element selbst zu speichern, anstatt globale Variablen zum Speichern von Informationen zu verwenden, um weniger zu erzeugen gekoppelte Funktionen.

Ansonsten gehen Sie den Code Zeile für Zeile hinunter:

Innerhalb der fill_ratingsFunktion:

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

Sie erstellen Elemente mit idAttribut, die mit einer Nummer beginnen. Dies ist vor HTML5 illegal. Stattdessen können Sie stattdessen Elemente mit der folgenden Syntax jQuery erstellen:

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

Der Hauptvorteil davon ist, dass Sie jetzt einen Verweis auf das gerade erstellte Element haben. Sie können es dann in ein Array verschieben, um es später wiederzuverwenden, anstatt neue jQuery-Objekte zu erstellen, indem Sie versuchen, dieselben Elemente später im Code erneut auszuwählen.

Variablendeklaration - meistens ein Stilvorschlag, Sie können jedoch mehrere vars vermeiden, indem Sie den Kommaoperator verwenden:

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

Nicht verwenden $(this), um Elementattribute zu erhalten :

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

ist etwa 97% langsamer als

this.id;

Letzteres ist weniger wortreich und viel effizienter.

In jedem der Event-Handler, die mit dem Ereignis verbunden sind#rating img - anstatt durch alle #rating imgElemente zu gehen, wie Sie es tun:

    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');
        }
    }

Wir können immer die bereits gegebenen Informationen (das aktuelle Element, das das Ereignis auslöst) für die Verarbeitung verwenden. Der obige Code kann beispielsweise wie folgt geschrieben werden:

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

Dadurch werden all diese Zeilen in eine Zeile reduziert und eine Variable entfernt. Die meisten anderen Event-Handler können auf ähnliche Weise neu geschrieben werden.

Der Typ-Zwang zuNumber kann ohne Verwendung ausgeführt werden Number- fügen Sie einfach ein Zeichen +vor die Variable. In Ihrem Fall ist dies völlig überflüssig, da Sie es Stringerneut zwingen, es zu einem anderen String in der Auswahl zu ändern, die an die jQuery-Auswahl übergeben wird

Vielen Dank für all die Informationen. Ich werde anfangen, das Plugin zu konvertieren und alles, was Sie erwähnt haben, zu aktualisieren bzw. zu reparieren. Ich habe Ihre Antwort akzeptiert, aber wenn jemand etwas hinzuzufügen hat - bitte tun Sie das! Andrew vor 9 Jahren 0
2
David d C e Freitas

Einige Automatisierungen können heutzutage viel helfen:

Googles eigener Closure Compiler von Google

ist ein Tool, mit dem Sie JavaScript schneller herunterladen und ausführen können. Es analysiert Ihr JavaScript, analysiert es, entfernt toten Code und schreibt neu und minimiert den Rest. Außerdem werden Syntax, Variablenverweise und -typen überprüft und es wird auf häufige JavaScript-Fehler hingewiesen.

Es gibt auch eine Online-Version dieses Tools, ein Firebug-Plugin, und es ist bereits Teil von PageSpeed, und es gibt auch einen JavaScript-Linter als Teil dieser Closure-Tools .