parseHint("Pro"): OOP und "unobstrusive" - Anfänger-Fragen

Liebe Javascript-Fortgeschrittenen,

da ich des öfteren in Antworten auf eigene Fragen im Forum und auch bei Lösungssuchen in anderen Forumsdiskussionen
auf mir bisher "spanisch" vorkommende Script-Notationen gestoßen bin, die sich in meiner Recherche
als Literale, Konstruktoren etc. entpuppten, habe ich mir jetzt einige OOP-Tutorials angeguckt und versuchsweise eines meiner Scripte umgeschrieben.
Vielleicht kann mir ja der ein oder andere auf die Sprünge helfen, mir sagen,
ob ich da totalen Murks angestellt habe, oder ob das eine Verbesserung vom Schema
var ..., var, ..., ... function..., function..., function...
hin zu unobstrusive scripting darstellt.

Meine konkreten Fragen:
(1) Ist das ein sinnvolles/gängiges Vorgehen, zunächst in einer namenlosen automatisch startenden Funktion (function(){...})() die Startbedingungen festzulegen und dann alle weiteren Variablen und Funktionen in einen Container
zu packen - also als Methoden, Eigenschaften einer Konstruktor-Fkt.?
(2) Wenn ich einem Element einen Event-Handler zuweisen möchte direkt, nachdem dieser im HTML-Baum erzeugt wurde und nicht erst nach vollständigem Laden aller Elemente, ist da die einzige Lösung, ein eigenes Script nur für diese Aktion zu schreiben und dieses direkt nach dem entsprechenden Element einzubinden (wenn ich auf Event-Handler im tag verzichten will)?

Im Folgenden das Script in 2 Schritten und die dazugehörige HTML (hier stark vereinfacht/auf das bzgl. Script relevante reduziert):
Startbedingungen:

/* - preload images - event handling definieren - neues Objekt als Instanz des Konstruktors (s.u.) - einge Werte und Bilder-Pfadnamen an öffentl. Methode des Konstruktors übergeben */ (function() {   var imgs = new Array();   for (i=0; i<10; i++) {     imgs[i] = new Image(); imgs[i].src = "bilder/strip" + (i+1) + ".jpg";   };   var General = { addEvent : function(element, eventName, function_) {   if (typeof(function_) == 'undefined') return false;   if (element.addEventListener) element.addEventListener(eventName, function_, false);   else if (element.attachEvent) element.attachEvent('on' + eventName, function_);   return true; }, addOnLoad : function(function_) {   if (document.readyState == 'loading') General.addEvent(window, 'load', function_);   else setTimeout(function_, 1); }   }   var dragger_1 = new Dragger();   General.addEvent(document, "mousemove", dragger_1.drag);   General.addEvent(document, "mouseup", function() {dragger_1.dragObjekt = false;});   // General.addEvent(document, "mousedown", function() {return false;});   document.onmousedown = function() {return false;};   if (document.all) { // warum klappt if(document.ondragstart) nicht??? document.ondragstart = function() {return false;};   };   if (document.all) { document.onselectstart = function() {return false;};   };   var Presets = { dia_strip           : "strip", id_pic_box     : "bild", images          : ["bilder/strip1.jpg", "bilder/strip2.jpg", "bilder/strip3.jpg", "bilder/strip4.jpg",    "bilder/strip5.jpg", "bilder/strip6.jpg", "bilder/strip7.jpg", "bilder/strip8.jpg",    "bilder/strip9.jpg", "bilder/strip10.jpg"], min_top         : 123, max_top         : 480, strip_map_top    : [472, 420, 367, 315, 262, 210, 157, 105, 52, 0], strip_map_bottom : [523, 471, 419, 366, 314, 261, 209, 156, 104, 51], strip_part_height : 50   };   // General.addOnLoad(function() {General.addEvent(document.getElementById('diabox'), "mousedown", function() {dragger_1.drag_start(this);});});   window.onload = function() {document.getElementById('diabox').onmousedown = function() {dragger_1.drag_start(this);};};   dragger_1.uebergabe(Presets); })();

Konstruktor:

function Dragger() {   var that = this;   this.uebergabe = function(Presets) { this.strip = Presets.dia_strip; this.pic_box = Presets.id_pic_box; this.images = new Array(); for (i=0; i<Presets.images.length; i++) {   this.images[i] = Presets.images[i] }; this.min_top = Presets.min_top; this.max_top = Presets.max_top; this.strip_map_top = new Array(); for (j=0; j<Presets.strip_map_top; j++) {   this.strip_map_top[i] = Presets.strip_map_top[i]; }; this.strip_map_bottom = new Array(); for (j=0; j<Presets.strip_map_bottom; j++) {   this.strip_map_bottom[i] = Presets.strip_map_bottom[i]; }; this.strip_part_height = Presets.strip_part_height;   }   this.dragObjekt = false;   var dragY = 0;   var posY = 0;   var stripY;   var aktuellePos;   var stripDownBusy = false;   var stripUpBusy = false;   this.drag_start = function(element) { that.dragObjekt = element; dragY = posY - that.dragObjekt.offsetTop;   };   this.drag = function(e) { posY = document.all ? (window.event.clientY + document.body.scrollTop  - document.body.clientTop) : e.pageY; stripY = parseInt(document.getElementById(that.strip).style.top); if (that.dragObjekt != false) {   if ((posY - dragY) > that.min_top && (posY - dragY) < that.max_top) { that.dragObjekt.style.top = (posY - dragY) + "px";   }   else if ((posY - dragY) <= that.min_top) { that.dragObjekt.style.top = that.min_top + "px"; if (stripDownBusy == false) {   stripDown(); }   }   else if ((posY - dragY) >= that.max_top) { that.dragObjekt.style.top = that.max_top + "px"; if (stripUpBusy == false) {   stripUp(); }   }   aktuellePos = posY - dragY;   aktualisiereBild();   return new Boolean(false); }   };   function aktualisiereBild() { var bild = document.getElementById(that.pic_box); for (i=0; i<that.images.length; i++) {   if ((that.dragObjekt.offsetTop - (that.min_top + 4)) >= (that.strip_map_top[i] + stripY - that.strip_part_height/2) && (that.dragObjekt.offsetTop - (that.min_top + 4)) <= (that.strip_map_bottom[i] + stripY - that.strip_part_height/2)) { bild.src = that.images[i];   } }   };   function stripDown() { stripDownBusy = true; if (that.dragObjekt == false) {   stripDownBusy = false;   return; } stripY = parseInt(document.getElementById(that.strip).style.top); if (stripY < 0 && aktuellePos <= 123) {   document.getElementById(that.strip).style.top = stripY + 2 + 'px';   aktualisiereBild();   window.setTimeout(function() {stripDown();}, 1); } else {stripDownBusy = false;}   };   function stripUp() { stripUpBusy = true; if (that.dragObjekt == false) {   stripUpBusy = false;   return; } stripY = parseInt(document.getElementById(that.strip).style.top); if (stripY > -107 && aktuellePos >= 480) {   document.getElementById(that.strip).style.top = stripY - 2 + 'px';   aktualisiereBild();   window.setTimeout(function() {stripUp();}, 1); } else {stripUpBusy = false;}   } }

HTML:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html> <?xml version="1.0" encoding="UTF-8"?> <head>   <script language="JavaScript" type="text/javascript" src="jscripts/galerie.js"></script>   <link rel="stylesheet" type="text/css" href="styles/galerie.css"> </head> <body>   <div>    <img src="bilder/strip.png" id="strip" />   </div>   <div id="diabox">    <img src="bilder/diarahmen.png" id="diarahmen" />   </div>   <div id="bildbox">    <img src="bilder/strip10.jpg" id="bild" />   </div> </body> </html>

So, wie es ist, funktioniert das Script wie es soll - ob es hinsichtlich objektorientierten Programmierens sinnvoll geschrieben ist, das frage ich Euch ;}
Vielen Dank im Voraus für jegliche Hilfe/Anregungen/...
Lieben Gruß

  1. (1) Ist das ein sinnvolles/gängiges Vorgehen, zunächst in einer namenlosen automatisch startenden Funktion (function(){...})() die Startbedingungen festzulegen und dann alle weiteren Variablen und Funktionen in einen Container zu packen - also als Methoden, Eigenschaften einer Konstruktor-Fkt.?

    Im Grunde kannst du sämtlichen Code so kapseln. Also auch den Konstruktor.

    Wenn du etwas nach außen geben willst, so gibst du etwas aus dieser Funktion zurück ober gibst das globale Objekt window in die Funktion hinein und erzeugst daran eine Eigenschaft. (In deinem Beispiel ist das aber wohl nicht nötig.)

    (2) Wenn ich einem Element einen Event-Handler zuweisen möchte direkt, nachdem dieser im HTML-Baum erzeugt wurde und nicht erst nach vollständigem Laden aller Elemente, ist da die einzige Lösung, ein eigenes Script nur für diese Aktion zu schreiben und dieses direkt nach dem entsprechenden Element einzubinden (wenn ich auf Event-Handler im tag verzichten will)?

    Es wäre die schlechteste Lösung, ständig nach allen möglichen Elementen Scripte einzubinden. Das hätte keinen Vorteil.

    Es reicht, DOMContentLoaded zu verwenden - da muss zwar das gesamte HTML geparst werden, aber das reicht i.d.R. aus, sofern selbiges nicht Megabyte groß ist.

    Aus Performance-Gründen ist es sinnvoll, <script src> ans Dokumentende zu verschieben. Dann ist ohnehin garantiert, dass du auf alle Elemente über das DOM zugreifen kannst.

    • preload images

    Das kann der Dragger übernehmen.

    • event handling definieren

    Das sollte ebenfalls der Dragger übernehmen.

    • einge Werte und Bilder-Pfadnamen an öffentl. Methode des Konstruktors übergeben */

    Diese kannst du auch direkt an den Konstruktor übergeben.

      var General = {
    addEvent : function(element, eventName, function_) {
      if (typeof(function_) == 'undefined')
    return false;
      if (element.addEventListener)
    element.addEventListener(eventName, function_, false);
      else if (element.attachEvent)
    element.attachEvent('on' + eventName, function_);
      return true;
    },

    Beachte, dass attachEvent nicht dasselbe macht wie addEventListener.

    addOnLoad : function(function\_) {   if (document.readyState == 'loading') General.addEvent(window, 'load', function\_);   else setTimeout(function\_, 1); }   }

    Hier solltest du wie gesagt auf DOMContentLoaded setzen.

      var dragger_1 = new Dragger();

      General.addEvent(document, "mousemove", dragger_1.drag);
      General.addEvent(document, "mouseup", function() {dragger_1.dragObjekt = false;});

    Diese Aufgaben gehören m.E. in den Dragger, oder wieso sollte das von Außen gemacht werden?

      if (document.all) { // warum klappt if(document.ondragstart) nicht???
    document.ondragstart = function() {return false;};

    ondragstart ist anfangs eine leere Eigenschaft. Sie enthält null o.ä. Wenn du if (document.ondragstart) prüft, bevor du der Eigenschaft einer Funktion zugewiesen hast, trifft die Bedingung natürlich nicht zu.

    Wenn du prüfen willst, ob der Browser den Event unterstützt, verwende:

    if ('ondragstart' in document) ...

      var Presets = { ... };
      dragger_1.uebergabe(Presets);

    Das kannst du wie gesagt einfach direkt dem Konstruktor als Initialisierungsdaten übergeben.

    new Dragger({ ... });

      this.drag = function(e) {
    posY = document.all ? (window.event.clientY + document.body.scrollTop  - document.body.clientTop) : e.pageY;

    Verwende bitte keine unbeteiligten Objekte wie hier document.all für Feature-Abfragen. Das ist sehr unzuverlässig. Frage direkt ab, ob e.pageY einen brauchbaren Wert enthält.

      return new Boolean(false);

    Wieso gibst du hier ein Boolean-Object zurück? Absicht? Ist dir der Unterschied zu einem normalen Boolean?Primitive bewusst?

    stripY = parseInt(document.getElementById(that.strip).style.top); if (stripY < 0 && aktuellePos <= 123) {   document.getElementById(that.strip).style.top = stripY + 2 + 'px';

    Wenn du mehrfach auf ein Element zugreifst, so führe »document.getElementById(that.strip)« einmal aus und speichere den Rückgabewert zwischen.

      window.setTimeout(function() {stripDown();}, 1);   window.setTimeout(function() {stripUp();}, 1);

    Ist dasselbe wie
    window.setTimeout(stripDown, 1);
    bzw.
    window.setTimeout(stripUp, 1);

    Wozu die Kapselfunktion?

    Ansonsten ist das schon recht sinnvoll strukturiert. Ich würde den Dragger aber noch weiter nach außen abschließen und wie gesagt die zusammengehörige Funktionalität wie das Registrieren der Event-Handler darin vereinigen.

    Mathias

    1. Hallo molily,

      vielen, vielen Dank für Deine ausführliche und hilfreiche Antwort; damit, dass
      sich jemand soviel Mühe gibt und sogar auf Details im Scripts eingeht, hab ich
      nicht gerechnet und bin also sehr positiv überrascht :-}
      Mit Deinen Hinweisen, Anregungen und Berichtigungen kann ich viel anfangen und
      freue mich schon darauf, mich mehr mit modernem Javascripting zu beschäftigen.

      Wieso gibst du hier ein Boolean-Object zurück? Absicht? Ist dir der Unterschied zu einem normalen Boolean?Primitive bewusst?

      Hm, hatte irgendwo (glaube in Felix Riesterers Fader-Framework) gelesen, der Konstruktor
      müsse ein Objekt zurückgeben, vielleicht hatte ich da was missverstanden; habe - ehrlich gesagt - darüber nicht weiter nachgedacht und bisher auch noch nicht genauer recherchiert. Das werde ich bald nachholen :]

      Ganz liebe Grüße und nochmals vielen Dank!

      1. Lieber parseHint("Pro"),

        Hm, hatte irgendwo (glaube in Felix Riesterers Fader-Framework) gelesen,

        *freu*

        Liebe Grüße,

        Felix Riesterer.

        -- ie:% br:> fl:| va:) ls:[ fo:) rl:| n4:? de:> ss:| ch:? js:) mo:} zu:)
        1. !

          *freu*

          hehe ;-}

          Na, dann an dieser Stelle auch noch einmal einen herzlichen Dank an Dich für
          einige sehr hilfreiche Antworten, die ich schon von Dir bekommen habe hier bei
          selfhtml und für das gute Tutorial - nachdem ich mich da so einigermaßen
          durchgewurschelt hatte, hab ich mir Mathias Schäfers Artikel "Organisation von
          JavaScripten" angeschaut und im Nachhinein noch mehr verstanden ;}
          selfhtml wird mir immer symphatischer!

          lieben Gruß

          1. ... - nachdem ich mich da so einigermaßen
            durchgewurschelt hatte, hab ich mir Mathias Schäfers Artikel "Organisation von
            JavaScripten" angeschaut und im Nachhinein noch mehr verstanden ;}
            selfhtml wird mir immer symphatischer!

            Dann dürfte dich vielleicht interessieren, dass ich seit geraumer Zeit an einem Nachfolgeartikel arbeite, siehe
            http://molily.de/weblog/javascript-organisation
            Ich bin nur noch nicht dazu gekommen, den SELFHTML-Artikel, der 2006 erschien und 2008 das letzte Mal aktualisiert wurde, dadurch zu ersetzen.

            Mathias

      2. Hm, hatte irgendwo (glaube in Felix Riesterers Fader-Framework) gelesen, der Konstruktor
        müsse ein Objekt zurückgeben, vielleicht hatte ich da was missverstanden; habe - ehrlich gesagt - darüber nicht weiter nachgedacht und bisher auch noch nicht genauer recherchiert.

        Das ist richtig. Der Konstruktor gibt ja automatisch ein Objekt zurück, nämlich die erzeugte Instanz. Wenn man explizit return verwendet, dann hat das nur einen Effekt, wenn der zurückgegebene Wert ein Object ist. Ein Boolean-Object zurückzugeben in bestimmten Fällen ist daher mehr ein Workaround, um irgendwie dem Außen mitzuteilen, dass etwas schiefgegangen ist. Das ist ein ziemlicher Sonderfall - in der Regel sind String-, Boolean- und Number-Objekte unnötig und es reichen ihre jeweiligen Primitives.

        Aber das besagte return new Boolean(false) steht bei dir gar nicht im Konstruktor, sondern in einer anderen normalen Funktion. Daher reicht da wohl ein einfaches return false aus.

        Mathias

  2. Hallo,

    Zur Struktur will ich jetzt mal nicht viel sagen, auf den ersten Bilck scheint sie mir gar nicht übel. Man könnte zunächst noch einiges am Code zu vereinfachen, anschliessend hat man dann einen noch besseren Überblick über die Stuktur, die dann man evtl. auch noch klarer bauen kann. Molily hat wohl schon das wichtigste zur Struktur gesagt.

    Die Kapselung als anonyme Funktion ist i.O., muss dann aber auch konsequent sein, z.B. hast du die Variable i in Zeile 5 wohl vergessen zu deklarieren, so dass sie zur globalen Variablen wird, was die Kapselung untergräbt.

    Was mir am meisten auffällt:

    Vergleiche wie in if(etwas == false) sind meistens unnötig und manchmal führen sie auch zu unerwarteten Ergebnissen.
    Mit true, false, 0, null, '' oder undefined sollte man entweder gar nicht vergleichen, d.h. einfach if(etwas) oder if(!etwas) notieren, oder aber – wenn man es denn sein muss – typgenau vergleichen mit den Operatoren === und !==.

    Wenn du mehrfach auf ein Element zugreifst, so führe »document.getElementById(that.strip)« einmal aus und speichere den Rückgabewert zwischen.

    Das würde ich noch allgemeiner formulieren: Eigentlich immer, wenn man auf ein Element, eine Eigenschaft oder einen berechneten Wert oder irgend etwas mehrfach zugreift, was nicht direkt im Scope liegt, sollte man es zwischenspeichern. Das erhöht die Performance und auch die Lesbarkeit bzw. Wartbarkeit des Codes. Z.B. hier:

    if (that.dragObjekt != false) {   if ((posY - dragY) > that.min_top && (posY - dragY) < that.max_top) {     that.dragObjekt.style.top = (posY - dragY) + "px";   }   else if ((posY - dragY) <= that.min_top) {     that.dragObjekt.style.top = that.min_top + "px";     if (stripDownBusy == false) {       stripDown();     }   }   else if ((posY - dragY) >= that.max_top) {     that.dragObjekt.style.top = that.max_top + "px";     if (stripUpBusy == false) {       stripUp();     }   }   aktuellePos = posY - dragY;   aktualisiereBild();   return new Boolean(false); }

    wird unnötigerweise mehfach posY - dragY berechnet und auch mehrfach auf Objekte ausserhalb des aktuellen Scope zugegriffen.
    Daher würde ich es lieber so notieren:

    if (that.dragObjekt) {   var dragObjektStyle = that.dragObjekt.style,       minTop = that.min_top,       maxTop = that.max_top,       dy = posY - dragY;   if (dy <= minTop) {     dragObjektStyle.top = minTop + "px";     if (stripDownBusy) { stripDown(); }   }   else if (dy >= maxTop) {     dragObjektStyle.top = maxTop + "px";     if (stripUpBusy) { stripUp(); }   }   else {     c.top = dy + "px";   }   aktuellePos = dy;   aktualisiereBild();   return false; }

    Ist das nicht viel übersichtlicher und leichter verständlich?
    Wenn man den Code so ein bisschen optimiert hat, sieht man oft auch strukturelle Ungereimtheiten viel besser. Ist es z.B. Absicht, dass die Funktion drag nur dann einen Rückgabewert liefert, wenn das dragObjekt nicht existiert?

    Hier:

      this.strip_map_top = new Array();   for (j=0; j<Presets.strip_map_top; j++) {     this.strip_map_top[i] = Presets.strip_map_top[i];   };   this.strip_map_bottom = new Array();   for (j=0; j<Presets.strip_map_bottom; j++) {     this.strip_map_bottom[i] = Presets.strip_map_bottom[i];   };

    kopierst du offenbar Arrays. Das geht einfacher mit der slice-Methode:

      this.strip_map_top = Presets.strip_map_top.slice(0);   this.strip_map_bottom = Presets.strip_map_bottom.slice(0);

    Ob das Kopieren überhaupt nötig ist, könnte man noch untersuchen.

    Gruß, Don P

    1. Liebe Don P,
      auch Dir vielen Dank für Deine ausführliche und überaus hilfreiche Atwort :)

      Die Kapselung als anonyme Funktion ist i.O., muss dann aber auch konsequent sein, z.B. hast du die Variable i in Zeile 5 wohl vergessen zu deklarieren, so dass sie zur globalen Variablen wird, was die Kapselung untergräbt.

      Stimmt. Das liegt wohl an der Macht der (noch präsenten) Gewohnheit.

      wird unnötigerweise mehfach posY - dragY berechnet und auch mehrfach auf Objekte ausserhalb des aktuellen Scope zugegriffen.

      OOPs, jetzt wo Du's sagst.. Ja, sollte mir wirklich angewöhnen, jede Zeile doppelt
      auf ihre Sinnhaftigkeit hin zu überprüfen.

      Lieben Gruß