try/catch - warum wird throw new Error() nicht ausgeführt und ist das die richtige Art, zurückzugeben was genau falsch ist?

[1unitedpower]

Hi ebody,

Ich habe nochmal eine andere, angepasste Variante erstellt: https://codepen.io/ebody/pen/oNGepjL

Großes Lob erstmal, es ist ein deutlicher Fortschritt gegenüber deiner initialen Version erkennbar. Ich habe noch ein paar Anmerkungen, eher allgemeiner Natur zum Error-Handling.

Ich bin ein großer Freund von Minimalismus, deshalb beginne ich mal mit einer rigoros gekürzten Variante deiner createList-Funktion.

function createList(movies) {
  return movies.map(movie => `<li>${movie}</li>`)
}

Das erste, das mir auffällt ist, dass hier der Kontextwechsel nicht beachtet wird. Wenn man HTML-Strings in JavaScript zusammenbaut, muss man genau wie in PHP darauf achten, dass HTML-Sonderzeichen entsprechend maskiert werden. Ansonsten hat man im besten Fall einen Defekt, der nie zu einem Fehler führt, und im schlimmsten Fall eine gravierende XSS-Sicherheitslücke. In PHP gibt es htmlspecialchars. In JavaScript gibt es soweit ich weiß, kein direktes Pendant dazu. Üblicherweise baut man DOM-Objekte zusammen und keine HTML-Strings. Der Einfachheit halber lass uns aber einfach annehmen, dass es eine escape-Funktion für JavaScript gäbe, dann kann man den Defekt beheben:

function createList(movies) {
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

Diese Funktion nimmt noch keine Plausibilitätsprüfung der Parameter vor. Spontan fallen mir drei Randfälle ein, über die es sich lohnt genauer nachzudenken:

1. Was ist wenn movies kein Array ist?
2. Was ist wenn ein Element aus movies kein string ist?
3. Was ist wenn das Array movies leer ist?

Wie man mit diesen Fällen umgeht ist eine reine Design-Entscheidung.

Die simpelste Herangehensweise ist einfach das garbage-in-garbage-out-Prinzip: man unternimmt einfach nichts. Klingt fahrlässig, ist in JavaScript aber gar nicht so unüblich. Besonders wenn es nur um so einfache Funktionen und leicht zu erkennende Randfälle geht. Der Nachteil ist natürlich, dass das zu unerwartetem Verhalten führen kann und das Defekte möglicherweise lange unentdeckt bleiben können.

Man könnte auch alle Fälle zur Laufzeit überprüfen und im Zweifelsfall Exceptions werfen:

function createList(movies) {
  if (!Array.isArray(movies))
    throw new TypeError("movies must be an array")
  if (!movies.every(movie => typeof movie === "string"))
    throw new TypeError("movies must contain only strings")
  if (movies.length === 0)
    throw new Error("movies must contain at least one element")
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

Der Vorteil ist, dass dem Entwickler, der diese Funktion mit fehlerhaften Werten aufruft, etwas mehr Informationen zur Verfügung stehen als ganz ohne Fehlerbehandlung. Ein Nachteil ist, dass ein schlechteres Verhältnis von Oberhead zu essentieller Programmlogik entsteht.

Die Ansätze lassen sich natürlich auch kombinieren, zum Beispiel könnte man auch die Design-Entscheidung treffen, dass der dritte Fall eigentlich ein regulärer Fall ist, und dass ein leeres Eingabearray zu einem leeren Ausgabe-Array führt.

function createList(movies) {
  if (!Array.isArray(movies))
    throw new TypeError("movies must be an array")
  if (!movies.every(movie => typeof movie === "string"))
    throw new TypeError("movies must contain only strings")
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

Manchmal verfeinert man den Ansatz auch und wirft domäne-spezifische Exceptions anstatt generischer Exceptions. Das macht es einfacher für den Anwender des Codes auf verschiedene Fehlerzustände zu reagieren:

class MoviesIsNotAnArray extends TypeError {}
class MoviesContainsNonStringType extends TypeError {}

function createList(movies) {
  if (!Array.isArray(movies))
    throw new MoviesIsNotAnArrayType("movies must be an array")
  if (!movies.every(movie => typeof movie === "string"))
    throw new MoviesContainsNonStringType("movies must contain only strings")
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

In diesem verkürzten Beispiel ist das zugegeben ein eher pathalogisches Szenario.

Alternativ, kann man mit Exceptions auch mit verschiedenen Rückgabewerten arbeiten, so wie du es auch schon tust.

class Result {}

class Success extends Result {
  #result;
  constructor(result) {
     this.#result = result
  }
  result() {
    return this.#result
  }
}

class Failure extends Result {
  #reason;
  constructor(reason) {
    this.#reason = reason
  }
  reason() {
    return this.#reason
  }
}

function createList(movies) {
  if (!Array.isArray(movies))
    return new Failure("movies must be an array")
  if (!movies.every(movie => typeof movie === "string"))
    return new Failure("movies must contain only strings")
  return new Success(movies.map(movie => `<li>${escape(movie)}</li>`))
}

Anders als bei Exceptions propagieren solche Rückgabewerte nicht durch den gesamten Programmfluss. Das kann ein Vorteil oder auch ein Nachteil sein. Im wesentlichen nehmen sich die beiden Ansätze aber nicht viel. In diesem konkreten Beispiel ist der Anteil an Boilerplate-Code aber deutlich höher.

Um den Boilerplate-Code zu reduzieren, könnte man anstatt auf die benutzerdefinierten Datentypen Result, Sucess und Failure auch auf Promises setzen:

async function createList(movies) {
  if (!Array.isArray(movies))
    throw new MoviesIsNotAnArrayType("movies must be an array")
  if (!movies.every(movie => typeof movie === "string"))
    throw new MoviesContainsNonStringType("movies must contain only strings")
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

Ich hab hier absichtlich den syntaktischen Zucker von async-Funktionen genutzt, um die Ähnlichkeit zwischen Exceptions und Promises zu verdeutlichen. Der Vorteil an diesem Ansatz ist, dass der Anwender des Codes selber entscheiden kann, ob er lieber mit Exceptions oder Promises arbeitet:

function testA() {
    createList(movies).then(doSomething).catch(handleFailure)
}

async function testB() {
   try {
       const result = createList(movies)
       doSomething(result)
   } catch (error) {
       handleFailure(error)
   }
}

Und zuletzt möchte ich dir nochmal das schon angesprochene TypeScript nahelegen. Alle bisherigen Ansätze reagierne nur auf Fehler, mit TypeScript kann früher ansetzen und es gar nicht zu Fehlern kommen lassen. Der Entwickler bekommt schon in seinem Editor angezeigt, dass etwas falsch gehen könnte und ist gezwungen pro-aktiv zu agieren, bevor es zu dem Fehler kommt.

function createList(movies : string[]) : string[] {
  if (movies.length === 0)
    throw new Error("movies must contain at least one element")
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

In dem oberen Beispiel, ist es unmöglich die Funktion mit einem Parameter aufzurufen, der kein reines Array von Strings ist. Folgendes würde alles von TypeScript schon zur Entwicklungszeit bemängelt:

createList(["foo", 42])
createList("foo")
createList({foo: "bar"})

Der nachfolgende Aufruf wäre zur Entwicklungszeit okay, würde aber zu einem Laufzeit-Fehler führen:

createList([])

Auch diesen Fehler könnte man zur Entwicklungszeit mit TypeScript abfangen:

type NonEmptyArray<a> = [a, ...a[]]
function createList(movies : NonEmptyArray<string>) : NonEmptyArray<string> {
  return movies.map(movie => `<li>${escape(movie)}</li>`)
}

Im Allgemeinen ist das Typsystem von TypeScript aber eher schwach, und man kann nicht alle Fehler so früh erkennen, deshalb ist das eher eine orthogonale Qualitätssicherungsmaßnahme.

Ich benutze alle der oben genannten Maßnahmen, je nach Kontext und je nach Code-Basis. Man sollte nicht unterschätzen, wie wertvoll eine konsistente Fehlerbehandlungs-Strategie in einer Code-Basis ist.