JavaScript Refactoring

Diese Woche bin ich über ein kleines .. simples JavaScript gestolpert. An dem möchte ich aber zeigen wie man auch sehr kleine Scripts grenzenlos schlecht machen kann. Das Script verwendet die Qooxdoo BaseLib.

Der Bug kam zu mir mit dem Hinweis daß das Script im IE8 und IE7 nicht funktioniert und das restliche JS der Seite lahmlegt.

Die Aufgabe des Scripts ist ein einfacher Container / Bildwechsel alle 5 Sekunden. Ein Container wird eingeblendet .. zwei andere werden ausgeblendet.

Orginalscript

Hier das Orginal Script mit Markup und JS.

Das kann im IE 7/8 nicht funktionieren .. weil der Browser noch nie document.getElementsByClassName unterstützt hat. Das muss man jetzt nicht wissen wenn man Junior Entwickler ist. Aber man sollte das doch auch im IE testen was man so baut. Außerdem werden da irgendwie viele Variablen übergeben .. und wer außerdem bei so ner kleinen Aufgabe 2 Schleifen ineinander baut und dann noch nen break verwendet .. okay .. das Ding stinkt .. und zwar auf den ersten Blick.

Wie gesagt .. aber es läuft in modernen Browsern klaglos.

Quickfix

Hier der Quickfix Code.

Als erstes musste natürlich ganz schnell ein Bugfix her .. keine Zeit sich groß Gedanken zu machen. Fixen .. online stellen war die Ansage.

Ich habe dem Container eine ID gegeben und hole mir das Element via ID ins JS und ermittle die Children. Die Basisfunktion habe ich dann für später aufgehoben. Außerdem kann ich ganz schnell noch 2 Variablen sparen. Die zwei Schleifen waren mir immer noch suspekt .. aber gut es soll ja erst mal nur tun.

Refactoring

Hier das Script Refactoring.

Nachdem online stellen habe ich es dann neu geschrieben. Mir gefiel der Ansatz im Markup 2 (oder mehr) Container auf hidden zu setzen und nur einen einzublenden und dies nicht per JS zu machen. Das wollte ich behalten. Alle andere habe ich gelöscht.

Auf beide Schleifen konnte ich verzichten .. ich muss ja immer nur ein Element einblenden und eins ausblenden. Alle anderen Elemente haben ja immer ein hidden. Das break konnte weg, eine zweite Zähler Variable war ebenso unnötig.

Das Interval wird gesichert indem es nur ausgeführt werden kann wenn die Variable auch wirklich ein Objekt enthält. Elemente und Children hole ich mir mit nativen JS Methoden ins Script die auch über alle Browser funktionieren.

Fertig. Elegant. Klein. Simpel.

Was lernen wir draus?

Erstmal das dies jedem passieren kann. Ich habe auch mal einen schlechten Tag oder stehe so unter Zeitdruck das ich so einen Müll schreibe .. Hauptsache es tut dann irgendwie.

Wenn man aber weiß das man da nur was hingemurkst hat muss man 1-2 Tage später (oder wenn wieder mehr Zeit ist) es so aufräumen und besser machen das es funktioniert.

Wenn man das nicht kann .. dann hat man doch zumindest das Gefühl es nicht wirklich gut gemacht zu haben. Dann geht man eben zu jemandem der besser JavaScript kann und lässt sich helfen .. und plötzlich lernt man vielleicht noch was.

Leave a Reply

Your email address will not be published. Required fields are marked *