Javascript:
let sec = (this.timer.time / 1000) % 60;
let min = ((this.timer.time / 1000) - (this.timer.time / 1000) % 60) / 60;
Vermeide redundanten Code, insbesondere bei Berechnungen. In diesem Beispiel ist es natürlich überschaubar, aber du teilst hier 3x time durch 1000, um aus deinem Timer, der wohl Millisekunden zählt, die Gesamtsekunden zu errechnen und das ist jedes Mal eine potentielle Fehlerquelle bzw erschwert die spätere Wartung. Ja, es geht in diesem konkreten Fall um Zeitberechnung, die sich nicht ändern wird, aber es geht mir eher um das Prinzip. Wiederkehrende Teilergebnisse, o.ä. sollte man stets in einer Variable kapseln, um sie in der Formel überall dort zu verwenden wo man sie benötigt. Ändert sich die Berechnung eines Teilergebnisses, ändert man es genau an einer Stelle, bei seiner einmaligen Zuweisung. Würde sich ungeachtet der Art der Berechnung in diesem Beispiel der Divisor 1000 ändern, müsstest du ihn an drei Stellen ändern und evtl gibt es noch eine vierte Stelle, die du übersehen hast.
Immer dann, wenn man merkt, dass man dasselbe zum zweiten, dritten, x-ten Mal tippt, sollte man sich fragen ob sich das besser kapseln lässt, zB als Variable
totalSeconds o.ä. In diesem kleinen Beispiel vielleicht nicht so ausschlaggebend, aber dennoch mindestens etwas für den Hinterkopf
Javascript:
this.position.x - this.gameWidth * 0.5
this.position.y - this.gameHeight * 0.5
So oder so ähnlich findet man überall im Code Koordinatenberechnungen. Es wäre auch hier zu überlegen, sowas zu kapseln und zB in den Tools in einer Funktion zu berechnen. Ggfs x und y als Koordinaten-Objekt handhaben und dann einfach sowas wie
GetGamePosition( objectposition ) aufrufen. Weniger x-y Gefummel und weniger fehleranfällig (zB x und y vertippen). Auch würde man damit den Faktor 0.5 leichter anpassen können, weil er genau an einer Stelle vorkommt - in der Funktion für die Koordinatenberechnung - statt an Dutzenden Stellen quer über den Code verteilt. Das schlägt in dieselbe Kerbe, die
@tollertyp schon gehauen hat.
Javascript:
switch (event.code) {
case "ArrowUp":
[..]
break;
}
In diesem Fall kommt der String zwar aus einem definierten Event, aber bei switch case mit Strings sollte man grundsätzlich darauf achten, dass zB "arrowUp" den case nicht triggern würde. Das kann zu ätzenden Fehler führen. Wenn nicht auszuschließen ist, dass der String auch abweichende Großschreibung haben kann - zB eine Benutzereingabe - sollte man den String im switch Statement zB mit
toUpperCase() normieren und alle cases entsprechend in Großbuchstaben schreiben. Ich mache das
grundsätzlich so, auch wenn ich 100%ig weiß wie die Strings reinkommen. Macht man es immer so, minimiert das das Risiko, dass es dieses eine Mal, wo man es sich sparen wollte, knallt.
Abgesehen davon ist es gewissermaßen ein ungeschriebenes Gesetz, stets den default case anzugeben, selbst wenn er abgesehen vom break nichts enthält. Einige Programmierungebungen tracken das auch und meckern einen fehlenden default case an, weil im Falle eines unerwarteten Werts der switch-Variable ein undefinierter Zustand entstehen kann.
Javascript:
enter() {
if (this.MENUSTATE == MENUSTATE.START1) {
this.game.start();
return;
}
if (this.MENUSTATE == MENUSTATE.HIGHSCORE) {
this.MENUSTATE = MENUSTATE.HIGHSCORESCREEN;
return;
}
if (this.MENUSTATE == MENUSTATE.WINSCREEN) {
this.MENUSTATE = MENUSTATE.HIGHSCORE;
return;
}
if (this.MENUSTATE == MENUSTATE.HIGHSCORESCREEN) {
this.MENUSTATE = MENUSTATE.HIGHSCORE;
return;
}
if (this.MENUSTATE == MENUSTATE.GAMEOVERSCREEN) {
this.MENUSTATE = MENUSTATE.HIGHSCORE;
return;
}
}
Es mag persönliche Präferenz sein, aber ich vermeide es, Funktionen mit mehreren return-Punkten zu schreiben, weil ich es für unübersichtlich halte. Mein Prof hat damals gesagt, dass eine Funktion am Ende returned und nicht mittendrin. Wenn überhaupt, mache ich sowas nur als shortcut ganz am Anfang, wenn eine
komplexe Funktion durch eine nicht erfüllte Vorbedingung gar nicht erst loslegen muss. Aber selbst dann kapsle ich den Inhalt der Funktion meistens in ein if und die Funktion läuft im Zweifelsfalle eben "leer" zum Ende. Entweder man macht aus den ifs ein if-else if- else Konstrukt oder ein switch und weist jeweils nur den MENUSTATE zu und nach dem if-else bzw switch kommt das return (explizit oder implizit). Ein Eingang und ei Ausgang anstelle von 6 Ausgängen. Auch hier wieder: Im kleinen Beispiel mag das halb so wild sein, aber wenn man sich das angewöhnt und auch bei komplexen Funktionen macht, schießt man sich eher früher als später ins Knie.
Javascript:
update(dt) {
if (!(this.isRunning)) { return; }
this.time += dt;
}
Hier zum Beispiel. Wieder 2 returns, das explizite return im if und das implizite return am Ende. Warum nicht so:
Javascript:
update(dt) {
if (this.isRunning) {
this.time += dt;
}
}
Das ist in meinen Augen auch leichter zu lesen. Wenn isRunning, mach was, wenn nicht, mach eben nichts.
Sicherlich gibt es einige, die in diesen Fällen anderer Meinung sind und das ist auch vollkommen legitim, da das meiste eher Fragen des persönlichen Stils sind.
Ansonsten sieht das schon ziemlich gut aus, wenngleich ich selbst eher selten javascript programmiere und das Projekt auch nur auf dem Tablet überflogen habe - keine Ahnung ob das überhaupt funktioniert
