Klassenerstellung / Erweiterung bereinigen

879
Ivo Wetzel

Ich habe mich lange gefragt, wie ich den untenstehenden Code bereinigen kann, ohne ihn weiter zu sprengen. Die Erweiterung der Klassen ist hier das Hauptanliegen, sie sieht ein bisschen zu magisch aus. Das liegt hauptsächlich daran, dass es mit den verschiedenen Fällen umgehen muss, und ich habe keinen Weg gefunden, den Code hier auf sinnvolle Weise zu reduzieren.

Vielleicht bin ich paranoid und der Code ist gut, aber ich würde trotzdem gerne ein Feedback dazu bekommen.

Der ursprüngliche Code und die Testfälle sind hier .

function is(type, obj) {
    return Object.prototype.toString.call(obj).slice(8, -1) === type;
}
function copy(val) { /* ...make shallow copy */ }
function wrap(caller, obj) {
    obj = obj || Function.call;
    return function() {
        return obj.apply(caller, arguments);
    };
}

function Class(ctor) {
    // ...default ctor stuff here....
    function clas(args) { /* ...actual instance ctor stuff... */}

    var proto = {};
    clas.init = wrap(ctor);

    // extend needs to be reduced in width, it easily goes over 80 columns
    // without some ugly if statements
    clas.extend = function(ext) {
        if (is('Function', ext)) {
            return ext.extend(proto); // holy closure!
        }

        for (var e in ext) {
            if (!ext.hasOwnProperty(e)) {
                continue; // a bit ugly imo, but it helps to prevent the indentation
                          // from blowing up
            }

            // this needs some refactoring, it creates bound and unbound
            var val = ext[e], func = is('Function', val);
            if (/^\$/.test(e)) { // statics
                proto[e] = copy(val);
                clas[e] = clas.prototype[e] = func ? wrap(clas, val) : val;

            } else if (func) {
                clas[e] = wrap(proto[e] = clas.prototype[e] = val);
            }
        }
        return clas;
    };

    // this could also need some clean up I suppose
    for (var i = ctor.hasOwnProperty('init') ? 0 : 1,
             l = arguments.length; i < l; i++) {

        var arg = arguments[i];
        is('Object', arg) ? clas.extend(arg) : arg.extend(clas);
    }
    return clas;
}
Antworten
19
Emulieren Sie keine Klassen. Dies ist ECMAScript. Raynos vor 9 Jahren 2
Vor einer Weile habe ich mir einiges vorgenommen, mein eigenes Klassendefinitionssystem zu erstellen, und wenn ich mich erinnere, waren meine Ziele ähnlich wie die, die Sie hier erreichen. Ihr sieht auf verschiedene Weise prägnanter aus, und JS-Entwicklung im Allgemeinen ist seitdem weit fortgeschritten, aber ich werde nachsehen, ob ich es ausgraben kann und alle Nuggets der Klugheit finden kann, die hier ebenfalls zutreffen könnten. Alles in allem finde ich, dass die letzte Revision ziemlich prägnant ist, obwohl sie zugegebenermaßen noch nicht vollständig bewertet wurde. Trotzdem mag ich was ich sehe :) TheXenocide vor 9 Jahren 0

2 Antworten auf die Frage

11
Eric Bréchemier

Ich würde vorschlagen, Kommentare hinzuzufügen (Javadoc-Stil, aber wahrscheinlich am besten, wenn viel leichter), um die Absicht jeder Methode zu beschreiben. Sogar die einfachsten. Ich fand es besonders nützlich in JavaScript zu beschreiben:

  • Art und Bereich der erwarteten Argumente
  • Welche Argumente sind optional und wie lauten die Standardwerte?
  • der Typ des Ergebniswerts, falls vorhanden
  • Was ist das Ergebnis, wenn die angegebenen Argumente nicht den Erwartungen entsprechen

Ansonsten stimme ich Ihrem Inline-Kommentar zu "dies erfordert etwas Refactoring, es werden gebunden und ungebunden". Der entsprechende Code sollte in eine separate Funktion extrahiert werden, wodurch auch die Verschachtelung reduziert wird, was eine Ihrer Sorgen zu sein scheint.

In Bezug auf den Code selbst würde ich wrap () in bind () umbenennen, um mit der in ECMAScript 5 hinzugefügten bind () - Funktion übereinzustimmen, und ich würde l in length in Länge umbenennen, da Buchstabe l leicht mit 1 verwechselt werden kann.

Ich habe Zweifel an diesem Teil des Codes:

if (is('Function', ext)) {
    return ext.extend(proto); // holy closure!
}

In meinem Verständnis: Sie prüfen zuerst, ob ext eine Funktion ist, bevor Sie die extend () -Methode aufrufen, aber:

  • extend () ist nicht in JavaScript definiert. Dies ist eine Ihrer benutzerdefinierten Methoden. Es kann also nicht garantiert werden, dass Sie diese Eigenschaft in der Funktion finden. Sie sollten wahrscheinlich einen Scheck hinzufügen.
  • Ich verstehe die Absicht nicht: Ein Inline-Kommentar würde helfen :)

Alles in allem finde ich es gut, dass der Code ein bisschen haarig ist, weil das Hinzufügen von Unterstützung für Klassen in JavaScript keine einfache Angelegenheit ist, aber viel mehr Inline-Kommentare (bis zu einem Kommentar pro Zeile Code für die komplexesten Dinge) würden sich verbessern die Lesbarkeit des Codes immens.

Die Umbenennung von "wrap" in "bind" scheint eine gute Idee zu sein. Das "ext.extend" erwartet eine weitere "Class" -Instanz, vielleicht wäre eine "ext instanceof-Klasse" hier die bessere Wahl. Das Hinzufügen von Kommentaren kann auch hilfreich sein. Mein Hauptproblem ist, dass es schwierig ist, sie kurz und einfach zu halten, genau wie es schwierig ist, Dinge zu benennen. Ich werde versuchen, es aufzuräumen und die Frage dann zu aktualisieren. Ivo Wetzel vor 9 Jahren 1
Sieht so aus, als wären Sie auf dem richtigen Weg :) Ich würde vorschlagen, für jede Funktion eine Beschreibung von Parametern hinzuzufügen, und für jedes Klugheitsintervall Inline-Kommentare wie die Verwendung eines regulären Ausdrucks: "if (/^\$/.test(i) ) ". Eric Bréchemier vor 9 Jahren 0
1
RobertPitt

Einige Tipps:

  • Stellen Sie sicher, dass Sie eine gute Einrückung verwenden
  • Erstellen Sie keine Funktion, die eine native Methode ausführt
    • also: if(is("Function",ext))wirdif(typeof ext == "Function")
  • Entfernen Sie unnötige Kommentare und Kommentare nur an der Spitze einer Entität.
  • Kürzen Sie Ihre Variablen nicht, da dies zu Problemen mit letzteren Entwicklern führt
  • Verwenden Sie eine strengere Typumwandlung:
    • if (!ext.hasOwnProperty(e)) wird if(ext.hasOwnProperty(e) == false)
  • Behalten Sie Ihre Bedingungen in den for-Schleifen in einer Zeile
  • Es ist nicht sinnvoll, einen Wert aus einem Array neu zuzuweisen, da Sie ihn an eine Funktion senden möchten
    • var arg = arguments[i];wird entfernt und arguments[i]an die Funktion gesendet

In Anbetracht der obigen Angaben würde Ihre Klasse so aussehen:

function Class(ClassBase)
{
    /*
        * Default Constructor, used to do XXX with YYY
    */
    var Arguments = args || [];

    function __Class(Arguments)
    {

    }

    var Prototype = {};
    __Class.Initialize= Wrap(ClassBase);


    /*
        * extend needs to be reduced in width, it easily goes over 80 columns
        * without some ugly if statements
    */

    __Class.Extend = function(ExtendableEntity)
    {
        if (typeof ExtendableEntity == "function")
        {
            return ExtendableEntity.extend(Prototype);
        }

        for (var Entity in ExtendableEntity)
        {
            if (ext.hasOwnProperty(Entity) == true)
            {
                var Value = ext[Entity]
                var _Function = (typeof Value == "function");

                if (/^\$/.test(Entity)) 
                {
                    Prototype[Entity] = Copy(Value);
                    __Class[Entity] = __Class.Prototype[Entity] = function ? Wrap(__Class, Value) : Value;
                }else 
                {
                    __Class[Entity] = Wrap(Prototype[Entity] = __Class.Prototype[Entity] = Value);
                }
            }
        }
        return __Class;
    }

    for (var i = ClassBase.hasOwnProperty('Initialize') ? 0 : 1, l = Arguments.length; i < l; i++)
    {
        (typeof Arguments[i] == 'object') ? __Class.Extend(Arguments[i]) : Arguments[i].Extend(__Class);
    }
    return __Class;
}
Genau das wollte ich nicht: D Jemand mit viel weniger Wissen über JS hat eine Antwort veröffentlicht. Das erste von `is (" Function ", ext)` ist ** nicht ** `typeof`. `typeof` ist vollständig defekt. ** Verwenden Sie ** niemals, um den Typ eines Objekts abzurufen. Argumente sind eine spezielle Variable innerhalb eines Funktionsbereichs. Das Starten von Methodennamen mit Großbuchstaben widerspricht der gängigen Praxis in JS Land. "hasOwnProperty" gibt nur "true" oder "false" zurück, also ist "!" in diesem Fall völlig in Ordnung. Zahnspangen in der nächsten Zeile können subtile Fehler verursachen. Weitere Informationen finden Sie unter http://bit.ly/JSGarden. Ivo Wetzel vor 9 Jahren 5
Abgesehen davon können Sie längere Variablennamen verwenden und es gibt keinen Sinn für var arg = arguments [i]. PS: Ihr Code schlägt alle Tests fehl :) Ivo Wetzel vor 9 Jahren 0
Das ist ein fairer Kommentar, `is ()` Funktion steht nicht zur Überprüfung zur Verfügung. Daher musste ich davon ausgehen (Sie haben auch in Kommentar * Typüberprüfung * angegeben), warum sollten Sie in JavaScript nicht die Großbuchstabenfunktion verwenden, wer hat Sie gesagt sollte nicht und gibt es einen grund? und der Grund für `== true` ist, dass es immer gut ist, sich an strenge Typ- und Wertüberprüfungen zu gewöhnen. Außerdem wurde mein Code nicht zum Laufen gebracht. Der Grund für das Format ist, ein Beispiel zu zeigen, wofür meine Kommentare waren. Pseudo nur Sir. RobertPitt vor 9 Jahren 0
`== true` ist in keiner Weise strenge Eingabe. `==` ** macht ** Zwang. Wenn Sie wirklich einen strengen Vergleich der Typen wünschen, hätten Sie `===` verwendet. Es kann auch eine gute Idee sein, die Quelle auf ausgelassene Methoden zu überprüfen, bevor man rät, was sie tun, nicht, dass ich hier plappere, aber ich vermute, dass die Leute, die den Code hier überprüfen, sich tatsächlich darum bemühen, wenn die Links vorhanden sind unter der Voraussetzung. In Bezug auf die Verwendung von Großbuchstaben verwenden die meisten großen JS-Projekte (und auch der größte Teil meines Codes) Großbuchstaben nur für benutzerdefinierte Typen. Ivo Wetzel vor 9 Jahren 5
@RobertPitt Es gibt eigentlich einen Grund, keine Großbuchstaben für reguläre Funktionsnamen zu verwenden: Dies ist eine Konvention, die angibt, dass die Funktion ein Konstruktor ist, dh sie soll mit dem Schlüsselwort new verwendet werden. Zum Beispiel neues Date () vs getDate (). Eric Bréchemier vor 9 Jahren 2
Ich würde definitiv * nicht * "unnötige Kommentare entfernen" und stimme ausdrücklich zu, dass "Kommentare nur an der Spitze eines Unternehmens stehen sollten". Kommentare sollten so nah wie möglich an der Sache liegen, die kommentiert wird (Prinzip der Nähe, was die Zuordnung erleichtert), schon deshalb, weil der Kommentar am besten als kommentierter Code auf dem Bildschirm angezeigt wird. Eric Bréchemier vor 9 Jahren 1