Hallo,

ich halte das Beispiel für problematisch, um daran über sprechenden Code zu diskutieren, schließlich sind schon zwei Funktionen vorgegeben:

collectAllEffectivelyContainedNodes(getActiveRange(), filter)

Ist schon eine imperative API, die ich so nicht entwerfen würde. Das halte ich für wenig »fluent«.

Man kann das durchaus funktional lösen, dann würde ich aber vorne anfangen. Für die schwächste Stelle der funktionalen Lösung halte ich ebenfalls .filter(function(value, i, arr) { return arr.slice(0, i).indexOf(value) == -1; }). Das ist sehr kompliziert zu verstehen und nicht sehr sprechend. Davon abgesehen ist es nicht sehr performant, wie du sagst. Beides hängt natürlich miteinander zu tun: Mehr algorithmischer Aufwand ist meist nicht intuitiv verständlich.

Deine Lösung ist da sicher verständlicher. Ich würde das Pferd aber anders herum aufzäumen: Was ist die Syntax, in der ich das Problem gerne ausdrücken würde, um den Kern des Problemes freizulegen? Mal ein Beispiel für eine Phantasiesyntax:

range = something.getActiveRange
nodes = range.containedNodes.editables.textNodes
return unless nodes.length
nodes.fontnames.unique.length > 2

editables und textNodes wären Filter, fontnames ein Mapper.

Das könnte man jetzt noch realistischer notieren (CoffeeScript-Rubyiesk):

range = something.getActiveRange()
nodes = range.containedNodes.select(:editable, :textNode)
return unless nodes.length
nodes.map(:fontname).unique().length > 2

Hier würde allerdings ein Short-Circuit verunmöglicht. Damit hätte ich erst einmal kein Problem. Erst wenn sich die map- und die unique-Operation als notorisch unperformant für die tatsächlichen Nodes bzw. Fontnamen erwiesen, würde ich optimieren. Etwa so:

nodes.unique?(:fontname, 2)

Wenn man die tatsächlich hässliche imperative Implementierung durch eine Abstraktion verstecken kann, wird der Code, der das Problem auf hoher Ebene formuliert, natürlich umso lesbarer. :)

Konkreter zu deinem Vorschlag: Allgemein tendiere ich auch dazu, mehr Variablen und Funktionen zu deklarieren, als nötig – selbst wenn sie eine Variable nur bis zur nächsten Zeile lebt und eine Funktion nur einmal aufgerufen wird. Ich verzichte darauf, eine möglichst ununterbrochene Kette von Funktionsaufrufen zu notieren. Das kann schließlich der JavaScript-Komprimierer machen, der merkt, dass Variable/Funktion nur einmal verwendet werden und daher weglassbar bzw. integrierbar sind.

Die Information, was eine kurze Funktion macht, muss man aber nicht unbedingt im Namen unterbringen und die Funktion dazu zentral notieren. Das macht deine Lösung eher umfangreicher und m.E. schwerer verständlich.

fontNames = nodes.map(function (node) { return node.fontName });

So etwas halte ich für intuitiv verständlich, wenngleich man natürlich eine bessere Syntax wie z.B. in Ruby haben könnte:

fontNames = nodes.map &:fontName

Allein CoffeeScript verbessert das Signal-Rauschen-Verhältnis schon stark, indem es die meisten Klammern unnötig macht:

fontNames = nodes.map (node) -> node.fontName

Wenn ich allerdings mit einer schlimmen API arbeiten muss, ergibt es vielleicht Sinn, das Implementierungsdetail durch eine ausgelagerte Funktion in der Formulierung des Problems zu verstecken. Ich würde die Information »node2fontname« erst einmal in einem schlichten Kommentar unterbringen:

fontNames = nodes.map(function (node) {
  // Get the node's font name
  return ugly.api.getEffectiveValue(node, "fontname", null, ugly.api.FOO | ugly.api.BAR, -2);
});

Sodass der Leser weiß, was passiert, und die nicht sprechende Implementierung ignoriert.

editableTextNodes = nodes.filter(function (node) { return isEditable(node) && node.nodeType == Node.TEXT_NODE });

Das halte ich auch für verständlich in einer Zeile mit anonymer Funktion. Wenn ich das sprechender gestalten wollte, dann indem ich an 'nodes' bzw. 'node' entsprechende Funktionalität definierte.

Mathias