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

[ebody]

Hallo,

ich habe hier eine Funktion mit try/catch, die die Parameter überprüfen soll. Mir ist wichtig, dass nicht nur throw new Error() ausgeführt wird, wo ich eine Fehlermeldung sehe, sondern dass die Funktion auch Fehler zurückgibt und sehen kann für welchen Parameter es Probleme gibt und was das Problem ist, so dass ich in einem späteren Script entsprechend darauf reagieren kann.

1. Warum wird console.log ausgeführt, aber nicht throw new Error()?

  catch (err) {
    console.log('err: ', err); // wird ausgeführt
    throw new Error(`Parameter problem...: ${err}`); // wird nicht ausgeführt
  }

2. Ist die Funktion ein guter/richtiger Weg oder wäre es z.B. gar nicht notwendig die try/catch Blöcke zu verschachteln und könnte es deutlich verkürzen?

Gruß ebody

[JürgenB]

Hallo,

2. Ist die Funktion ein guter/richtiger Weg oder wäre es z.B. gar nicht notwendig die try/catch Blöcke zu verschachteln und könnte es deutlich verkürzen?

ich verwende try/catch um Syntaxfehler abzufangen, z.B. ob der Browser modernes Javascript kann.

Parameter prüfe ich, indem ich abfrage, ob der Inhalt meinen Erwartungen entspricht.

Gruß
Jürgen

[dedlfix]

Tach!

1. Warum wird console.log ausgeführt, aber nicht throw new Error()?

Ich kann das Problem nicht nachvollziehen. Es gibt auch keinen in deinem Code ersichtlichen Grund, warum das throw nicht ausgeführt werden sollte.

2. Ist die Funktion ein guter/richtiger Weg oder wäre es z.B. gar nicht notwendig die try/catch Blöcke zu verschachteln und könnte es deutlich verkürzen?

Die Frage ist, ob du an der Stelle den Fehler ausgeben musst und ihn nicht einfach dem Aufrufer überlassen kannst.

dedlfix.

[dedlfix]

Tach!

Ich kann das Problem nicht nachvollziehen.

Ah, ich hab den Rest von deinem Code gefunden. Es liegt nicht am throw, sondern dass der catch-Block nicht ausgeführt wird, weil deine Bedingung Object.values(objParameters).includes(false) nicht wahr wird. Alle anderen Exceptions sind bereits behandelt worden.

Generell ist zu sagen, dass du zu viel try-catch verwendest. Geworfen werden sollten nur Ausnahmen. Deine Prüfungen hingegen sind normaler Programmfluss innerhalb check-Funktion. Es besteht kein Grund, sie in try-catch-Blöcke einzurahmen, in denen nur du selbst Exceptions wirfst.

Das return im finally ergibt auch keinen Sinn. Du kannst nicht gleichzeitig eine Funktion mit einer Exception und normalem Ergebnis beenden. Deine Check-Funktion gibt einen Status zurück. Sie läuft (normalerweise) intentionsgemäß und es gibt darin keine Bedingung, die einen außergewöhnlichen Abbruch erfordert. Du kannst mit den jetzigen Prüfungen komplett auf Exceptions verzichten.

dedlfix.

[Rolf B]

Hallo ebody,

ein try-Block, der nichts weiter tut als etwas zu prüfen und im Fehlerfall eine Exception zu werfen, gefolgt von einem catch-Block, der diese Exception gleich wieder wegschnappt, ist Unfug. So ist das nicht gedacht.

Deine check-Funktion sollte überhaupt kein catch enthalten, sondern nur prüfen und bei Fehlern die Exception werfen.

objParameters ist ein komisches Viech - du verwendest es offenbar aus einem Elternkontext heraus (denn es gibt kein let/const und keine Initialisierung dazu), und dann gibst Du es zurück. Also - entweder ist es lokal und Du gibst es zurück, oder es ist aus einem äußeren Kontext und Du veränderst es direkt. Beides geht, beides kann sinnvoll sein.

Deine check-Funktion sollte auch nicht beide Parameter prüfen. Wenn Du das tust, schreibst Du für jede Methode eine eigene Checkfunktion. Du solltest das eher so designen:

class Foo {
   function someMethod(name, age) {
      validateString("Foo:someMethod:name", name);
      validateInt("Foo:someMethod:age", age);

      // normale Verarbeitung
   }
}

function validateString(name, value) {
   if (typeof value != 'string')
      throw new TypeError(name + ": Ungültiger Typ, String erwartet, " + (typeof value ) + " erhalten");
}

try {
   let f = new Foo();
   f.someMethod("Rolf", "zu alt");
}
catch (err) {
   if (err instanceOf TypeError) {
      // Fehler protokollieren
   }
}

Deine Methode validiert und schmeißt die Exception. Fangen sollte sie sie nicht, das macht der Aufrufer. Der try-Block darf auch gerne mehr tun; den TypeError solltest Du nur werfen, wenn eine Fortsetzung des Codes keinen Sinn ergibt. WENN eine sinnvolle Fortsetzung möglich ist, solltest Du keinen Error werfen.

Du kannst auch eine Klasse von TypeError ableiten (glaube ich) und eigene Propertys hinzufügen, z.B. um die von mir im "name" Parameter angedeutete Fehlerstelle in einem separaten Property der Exception zu speichern.

Eine vollständige Analyse aller Fehlerstellen bekommst Du so natürlich nicht. Die erste Exception bricht das ab. Wenn Du mehrere Fehler sammeln willst, brauchst Du eine Aggregierung. Das könnte man so machen:

   function someMethod(name, age) {
      throwErrors("Foo:someMethod(name,age)",
         validateString("name", name);
         validateInt("age", age);
      );

      // normale Verarbeitung
   }

function validateString(name, value) {
   if (typeof value == 'string')
      return null;
   return name + ": Ungültiger Typ, String erwartet, " + (typeof value ) + " erhalten");
}


function throwErrors(method, ...validationMessages) {
   allMessages = validationMessages.filter(msg => !!msg);
   if (allMessages.length > 0) {
      // die gesammelten Messages sinnvoll aufbereiten
      throw new TypeError(...)
   }
}

D.h. die Validierungsmethoden geben null zurück, wenn alles ok ist, und sonst einen Error. Und throwOnError bekommt den Methodennamen sowie die Ergebnisse (per ... Rest-Parameter als Array). Im ersten Schritt schmeißt sie die leeren Ergebnisse weg (!! ist eine doppelte Verneinung, d.h. aus null wird false und der Rest wird true. Du könntest auch ausführlicher und lesbarer msg => msg != null && msg != "" schreiben). Ist dann noch was übrig, baust Du eine Sammelmeldung zusammen und wirfst sie in einem TypeError.

Dort, wo validiert wird, steht nur das nötigste. Prüfung der Parameter und Integration der Meldungen erfolgt in der Helper-Funktion, Abfangen der Exception beim Aufrufer. Man muss nicht alles wegabstrahieren.

Man kann natürlich brutal mit JavaDoc-Kommentaren als Pseudo-Attribute herangehen, und in someMethod.toString() herumparsen (was Dir den Quellcode liefert). Aber ich würde mir damit nicht die Programmiererseele beflecken wollen.

Wie schon mal gesagt: Wenn dir Typprüfungen wichtig sind, entwickle mit TypeScript.

Rolf

[Mitleser 2.0]

ein try-Block, der nichts weiter tut als etwas zu prüfen und im Fehlerfall eine Exception zu werfen, gefolgt von einem catch-Block, der diese Exception gleich wieder wegschnappt, ist Unfug. So ist das nicht gedacht.

Man könnte "JSON.parse" als eines der wenigen Gegenbeispiele anführen…

[Rolf B]

Hallo Mitleser,

ein try-Block, der nichts weiter tut als etwas zu prüfen und im Fehlerfall eine Exception zu werfen, gefolgt von einem catch-Block, der diese Exception gleich wieder wegschnappt, ist Unfug. So ist das nicht gedacht.

Man könnte "JSON.parse" als eines der wenigen Gegenbeispiele anführen…

Nein, sondern als Pro-Beispiel. JSON.parse entspricht der Foo.prototype.someMethod Methode Funktion aus meinen Ausführungen.

Und derjenige, der JSON.parse nutzt, ist der Aufrufer, der die Exception fangen muss. Einen Syntax-Error wegen fehlerhaften JSON sollte man zügig fangen, damit man ihn von anderen SyntaxError unterscheiden kann (nachträglich im stack-Property des Errors rumzufuhrwerken, um zu schauen, ob JSON.parse drin steht, halte ich für keine sinnvolle Lösung).

Strategien für's Errorhandling - gerade wenn viel Code läuft und ggf. größere Blöcke transaktional behandelt werden müssen, sind eine Wissenschaft für sich.

Rolf

[Mitleser 2.0]

Nein, sondern als Pro-Beispiel. JSON.parse entspricht der Foo.prototype.someMethod Methode Funktion aus meinen Ausführungen.

Und derjenige, der JSON.parse nutzt, ist der Aufrufer, der die Exception fangen muss. Einen Syntax-Error wegen fehlerhaften JSON sollte man zügig fangen, damit man ihn von anderen SyntaxError unterscheiden kann (nachträglich im stack-Property des Errors rumzufuhrwerken, um zu schauen, ob JSON.parse drin steht, halte ich für keine sinnvolle Lösung).

Strategien für's Errorhandling - gerade wenn viel Code läuft und ggf. größere Blöcke transaktional behandelt werden müssen, sind eine Wissenschaft für sich.

Okay, da kann ich soweit folgen. Ich hatte es zu schnell und einfach auf "try / catch ist böse" gemünzt und nicht konkret auf den Punkt. Sorry.

[ebody]

Vielen Dank für eure Hilfe und Tipps. Ich habe hier jetzt ein Beispiel erstellt: https://codepen.io/ebody/pen/bGoRbPR

Etwas anders, aber es erfüllt genau den Zweck, den ich haben möchte und alles ohne try/catch. So sollte es eigentlich funktionieren, das ich für verschiedene Methoden Parameter prüfen kann, einen finalen Status erhalte und auch bei Bedarf auf Details zugreifen kann, wie bei welchem Parameter gab es Probleme und bei welcher Überprüfung.

Die Methoden sollen immer ein Objekt zurückgeben, damit ich in späteren Skripten auf deren Status reagieren kann und entweder auf Fehler Details oder was die Methode im Erfolgsfall zurückgeben soll, zugreifen kann.

Ich bin gerade etwas platt und muss mal Pause machen. Evtl. stoße ich auch noch auf ein paar Dinge, die ich noch nicht bedacht habe.

Gruß ebody

[Rolf B]

Hallo ebody,

ich bin mit dieser Implementierung gar nicht zufrieden. Aber ich bin auch nicht der Architektengott, das mag einfach mein persönliches Missfallen sein.

Meine Kritikpunkte wären:

• Die response-Eigenschaft der Rückgabe ist mehrdeutig. Bei status==true ist es die fachliche Antwort, bei status==false das Fehlerobjekt. Das ist schwierig zu handhaben. Vor allem hat die response-Eigenschaft keinen klaren Datentyp.

• Deine Checks verlieren den Bezug zum geprüften Ding. Sprich: Wenn Du die Fehlermeldungen ausgibst, weißt Du zwar, dass da etwas Kein String war. Aber wenn die Methode 3 Strings erwartet, weißt Du nicht, welcher Parameter das war. D.h. du musst eine Kontextinformation mitgeben, z.B. den Parameternamen, und die Check-Methode muss diese Info in die Message einbauen.

• Die check-Methode enthält einen dicken Switch über den type Parameter. Es gibt keinen gemeinsamen Code für alle type-Werte. Du solltest eine Methode pro type schreiben: checkTypeArray, checkEmptyArray, checkTypeString, checkEmptyString, checkTypeInt.

• Die Bezeichnung "emptyArray" ist irreführend. Der Check "typeString" legt nahe: Es wird erwartet, dass der Typ "String" vorliegt. Der Check "emptyArray" würde, bei gleicher Lesart, nahelegen, dass ein leeres Array erwartet wird. Es ist aber genau andersrum. Der Typ des Checks sollte also "notEmptyArray" heißen.

• Es ist sehr umständlich, dass Du in objCheck ein Array von Objekten aufbaust. Das macht die status-Prüfung kompliziert. Du verwendest die Keys dieser Objekte nicht. Du könntest also einfach ein Array aus allen Prüfungen erzeugen, ohne Keys

• Die status-Methode des parameter-Objekts finde ich unverständlich. Ich würde sie IsSuccess oder so nennen.

• Die Check-Implementierung ist aufwändig. Natürlich kann man Objekte mit status und message erzeugen (nicht msg), aber wenn Du aufwändigeren Code hast, sammeln sich dafür ganz ordentliche Laufzeiten an. Ein Objekt zu erzeugen ist nicht billig. Auf diesem tiefen Level ist eine Optimierung durchaus sinnvoll; ich würde aus den check-Methoden null oder undefined zurückgeben, wenn die Check erfolgreich war, und eine Fehlermeldung, wenn er es nicht war.

createList(movies, str) {
   const objCheck = [
      parameter.checkTypeArray("movies", movies),
      parameter.checkNotEmptyArray(movies),
      parameter.checkTypeString("str", str)
   ];

   if (!parameter.IsSuccess(objCheck))
      return {
         status: false,
         checkResult: objCheck;
      };

   const movieList = arr.map(item => `<li>${item}</li>`);
   return {
      status: true,
      response: movieList
   }
}

Und noch ein Tipp:

let hasError = checkResults.map(result => result.status).includes(false);

lässt sich auch so bauen (achte auf die Negierung vorne):

let hasError = !checkResults.every(result => result.status)

Die zweite Zeile kommt ohne ein Temp-Array aus. Die Array-Methoden some und every sind oft nützliche Helfer.

Rolf

[ebody]

Hallo Rolf,

vielen Dank für dein Feedback.

Die response-Eigenschaft der Rückgabe ist mehrdeutig. Bei status==true ist es die fachliche Antwort, bei status==false das Fehlerobjekt. Das ist schwierig zu handhaben. Vor allem hat die response-Eigenschaft keinen klaren Datentyp.

Der Gedanke war, dass jede Methode immer ein Objekt mit den gleichen Keys zurückgibt und weiß, response gibt mir entweder Fehler Details oder was ich von der Methode haben möchte. Wäre die gängige Praxis, im Fehlerfall den Key error, statt response zu verwenden?

Deine Checks verlieren den Bezug zum geprüften Ding. Sprich: Wenn Du die Fehlermeldungen ausgibst, weißt Du zwar, dass da etwas Kein String war. Aber wenn die Methode 3 Strings erwartet, weißt Du nicht, welcher Parameter das war. D.h. du musst eine Kontextinformation mitgeben, z.B. den Parameternamen, und die Check-Methode muss diese Info in die Message einbauen.

In der (und in jeder anderen) Methode würde ich das Array arrCheck (hieß zuvor objCheck) erstellen. Darin lege ich fest welcher Parameter auf was geprüft werden soll. Das erste Element bezieht sich auf den ersten Parameter, das 2. Element auf den 2. Parameter usw. Jedes Element kann mehrere Prüfungen enthalten.

Die check-Methode enthält einen dicken Switch über den type Parameter. Es gibt keinen gemeinsamen Code für alle type-Werte. Du solltest eine Methode pro type schreiben: checkTypeArray, checkEmptyArray, checkTypeString, checkEmptyString, checkTypeInt.

type bestimme ich selbst. Ich übergebe ihn als Parameter für check(). Z.B. parameter.check(arr,'typeArray').

Die Bezeichnung "emptyArray" ist irreführend. Der Check "typeString" legt nahe: Es wird erwartet, dass der Typ "String" vorliegt. Der Check "emptyArray" würde, bei gleicher Lesart, nahelegen, dass ein leeres Array erwartet wird. Es ist aber genau andersrum. Der Typ des Checks sollte also "notEmptyArray" heißen.

Schaue ich nochmal und passe es an.

Es ist sehr umständlich, dass Du in objCheck ein Array von Objekten aufbaust. Das macht die status-Prüfung kompliziert. Du verwendest die Keys dieser Objekte nicht. Du könntest also einfach ein Array aus allen Prüfungen erzeugen, ohne Keys.

Wenn ich über response in einem Fehlerfall das Objekt erhalte, zeigen die Keys, um welche Prüfung es ging.

Die status-Methode des parameter-Objekts finde ich unverständlich. Ich würde sie IsSuccess oder so nennen.

Ich überleg noch mal einen anderen Namen.

Die Check-Implementierung ist aufwändig. Natürlich kann man Objekte mit status und message erzeugen (nicht msg), aber wenn Du aufwändigeren Code hast, sammeln sich dafür ganz ordentliche Laufzeiten an. Ein Objekt zu erzeugen ist nicht billig. Auf diesem tiefen Level ist eine Optimierung durchaus sinnvoll; ich würde aus den check-Methoden null oder undefined zurückgeben, wenn die Check erfolgreich war, und eine Fehlermeldung, wenn er es nicht war.

Anstatt, dass parameter.check() Objekte zurückgibt, könnte man ja auch nur true/false zurückgeben. Da ich den Key wie z.B. typeArray habe, weiß ich was überprüft wurde und funktioniert oder nicht. In einem weiteren Script könnte ich darauf reagieren und jeweils eine individuelle Fehlermeldung erstellen. Weil, ich brauche ja nicht immer einen Text (message) für das Überprüfungs für jedes Ergebnis.

let arrCheck = [
           {
             typeArray: parameter.check(arr,'typeArray') // true/false,
             emptyArray: parameter.check(arr,'emptyArray'),
           },
           {
             typeString: parameter.check(str,'typeString')
           }     
         ]

Bzgl. Laufzeit. Die Überprüfungen in const parameter werden ja nur einmalig eingebunden und aus verschiedenen Methoden auf nicht alle, sondern gezielte Prüfungen zugegriffen. Grob geschätzt könnten sich max. 50 Prüfungen in dem Objekt mit der Zeit ansammeln. Wäre das wirklich so deutlich spürbar?

Und noch ein Tipp:

let hasError = !checkResults.every(result => result.status)

Das bezieht sich auf diesen Schnippsel oder?

status: function status(arrCheck){
    return arrCheck.map(obj => Object.values(obj).map(objCheckValue => objCheckValue.status).includes(false)).includes(true);
  },

Auf jeden Fall auch Danke für diesen Tipp. every kannte ich gar nicht. Ich muss mal schauen, ob ich das verkürzen kann. Weil ich ja wahrscheinlich dieses Array in dieser Form nutzen werde.

let arrCheck = [
           {
             typeArray: parameter.check(arr,'typeArray'),
             emptyArray: parameter.check(arr,'emptyArray'),
           },
           {
             typeString: parameter.check(str,'typeString')
           }     
         ]

Gruß ebody

[Rolf B]

Hallo ebody,

Der Gedanke war, dass jede Methode immer ein Objekt mit den gleichen Keys zurückgibt

Ich bezweifle, dass die Wiederverwendung einer Eigenschaft für unterschiedliche Zwecke sinnvoll ist. Letztlich ist es aber deine Entscheidung.

Zum Thema "Verlust des Bezugs": Das arrCheck-Array, das die Prüfobjekte enthält, steht nach der Rückkehr aus der Methode für sich. Du musst für die „Berichterstattung“ die Bezug zu den Parametern rekonstruieren. Besser ist es, wenn arrCheck diesen Bezug enthält. Wie auch immer man das möglichst leichtgewichtig realisiert; du möchtest ja nicht für jeden Methodenaufruf eine monumentale Datenstruktur mit Prüfergebnissen aufbauen. Im Gegensatz zum parameter-Objekt, wo Deine check-Methoden sitzen, wird arrCheck ja wirklich für jeden Aufruf gebildet.

Im parameter-Objekt ist es dagegen wurscht, ob Du eine oder 100 Methoden einbaust. Die Verteilung der Tests auf mehrere Methoden hat aber deshalb Laufzeitvorteile, weil switch - im Gegensatz zur switch-Implementierung beispielsweise in C oder C++ - von oben nach unten prüft. X Cases ergeben durchschnittlich X/2 Abfragen pro check-Aufruf. Das Auffinden einer Methode im Methodenverzeichnis eines Objekts erfolgt dagegen über einen Hash und gelingt deshalb mehr oder weniger zeitverlustfrei. Das ist aber nicht der Hauptgrund für eine Aufteilung. Sondern: (a) ist es weniger fehleranfällig, weil ein vertippter Methodenname im Log sofort auffällt und (b) sind viele kleine Methoden besser lesbar als eine große.

Wenn ich über response in einem Fehlerfall das Objekt erhalte, zeigen die Keys, um welche Prüfung es ging.

Aha. Soso. Und dann verwendest Du in der status-Methode Object.values(obj) und guckst Dir die Keys gar nicht erst an.

Es ist auch irrelevant für die Fehlerberichterstattung. Aber wenn Du unbedingt willst, hänge den Namen der Prüfung in das Objekt, das von der Prüfmethode zurückgeliefert wird. So, wie Du es jetzt machst, musst Du den Prüftyp zweimal hinschreiben - einmal als Key im Objekt und einmal als Parameter der check-Methode.

Jetzt, wo ich mir das ein paarmal durch den Kopf habe gehen lassen - Du könntest auch mit Prototypen arbeiten. Wenn Du N Prüfungen hast, gibt es - so wie es jetzt gebaut ist - genau 2N mögliche Ergebnisobjekte, denn der Namen des geprüften Parameters steht ja gar nicht drin. Statt diese Objekte bei jeder Prüfung neu zu generieren, kannst Du sie einmalig erzeugen und dann für das eigentliche Ergebnis als Prototyp verwenden. Das reduziert den Aufwand für die Ergebniserzeugung deutlich; die meiste Arbeit findet nur einmal statt, bei der Erzeugung des parameter-Objekts.

https://jsfiddle.net/Rolf_b/py3ajsu8/

Nur mal so als Idee. Das Fiddle arbeitet gnadenlos funktional, mit Closures, Prototypen und selbstdefinierten Properties. Kann man mit class-Syntax vermutlich nochmal verschönern.

Rolf

[ebody]

Hallo Rolf,

Aha. Soso. Und dann verwendest Du in der status-Methode Object.values(obj) und guckst Dir die Keys gar nicht erst an.

In parameter.status wird nur der finale Status von allen Prüfungen gespeichert. Wenn ich über response in einem Fehlerfall das Objekt erhalte, zeigen die Keys, um welche Prüfung es ging. Hier kann ich die Keys dann auch nutzen.

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

Statt Switch verwende ich wie von dir empfohlen Funktionen. Das Array für den Aufruf der Check Funktionen enthält keine Objekte mit Keys mehr, sondern Arrays, um nicht 2x den Checknamen verwenden zu müssen und noch andere Änderungen. Weiter unten im Script sieht man, wie man auf Fehler reagieren könnte und Details erhält.

Damit erhalte ich alles was ich wollte:

1. Parameter über externe Funktion prüfen, um Code nicht zu wiederholen.

2. Script soll manchmal abgebrochen werden, wenn es Parameter Fehler gab.

3. Methode soll true/false als Feedback zurückgeben, um darauf reagieren zu können.

4. Alle Fehler erhalten und wissen welcher Parameter Fehler enthält.

5. In der Konsole alle Fehler sehen.

Gruß ebody

[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.

[Rolf B]

Hallo 1unitedpower,

lass uns aber einfach annehmen, dass es eine escape-Funktion für JavaScript gäbe,

nein, lass uns das nicht tun. Es gibt nämlich eine Funktion dieses Namens, aber sie escaped für URIs und nicht für HTML.

Man könnte:

• mit movie.replace(/<>&/g, (match) => ...) die drei kritischen Zeichen ersetzen
• das auch mit drei Replace tun, die nacheinander <, > und & durch &lt; &gt; und &amp; ersetzen

Oder man nutzt aus, dass man einen Browser hat: man erzeugt ein temporäres li-Element erzeugen, weist dem den Text als textContent zu und liest das outerHTML wieder aus. Solange man das li nicht ins DOM hängt, fliegt es danach automatisch in den Orkus.

let tempElem = document.createElement("li");
return movies.map(movie => {
                              tempElem.textContent = movie;
                              return tempElem.outerHTML;
                           });

Das kann man auch mit einem Kommaausdruck statt geschweifter Klammern lösen, aber das finde ich ziemlich schlecht lesbar.

createList(movies).then().catch()

Kann man machen, aber dann muss man auch dringend auf das Timing achten. Ein Promise erzeugt immer einen Eintrag in der Microtask-Queue, d.h. der Code im then läuft nie synchron, auch dann nicht, wenn in der async-Funktion keine wirkliche Asynchronität besteht. Mit await statt then kapselt man das besser, man behält auch in Methoden das this, ohne sich Mühe geben zu müssen. Ich bezweifle aber schon, dass eine Promise-Konstruktion für die Parametervalidierung von Funktionen/Methoden tatsächlich empfehlenswert ist.

Rolf

[ebody]

Hallo 1unitedpower,

viele Dank für die guten Tipps. Auch hier war wieder einiges neues für mich dabei.

Vielleicht habe ich etwas übersehen, aber wenn nicht würde man mit keinem der Beispiele auf einen bestimmten Parameter reagieren können und auch kein "Fehler Protokoll" erhalten, wo jeder Fehler für jeden Parameter aufgelistet ist.

Beispiel:

1. Ich rufe eine Methode auf, die 2 Parameter benötigt.
2. In einem späteren Script verwende ich Werte aus 2 input Feldern und übergebe diese an die Methode.
3. In diesem Script benötige ich jetzt ein Feedback der Methode, ob alles geklappt hat oder nicht. Wenn nicht, möchte ich wissen, ob es den ersten Parameter (und damit das erste input Feld) betrifft oder den 2. Parameter und was der Fehler ist. So könnte ich dann z.B. das erste input Feld formatieren und/oder einen passenden Hinweis Text dafür ausgeben.

Gruß ebody

[dedlfix]

Tach!

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>${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?

Wenn man den Minimalismus weiterbetreibt, könnte man auch noch Zusatzfragen stellen.

Zu 1: Ist es denn überhaupt möglich, dass movies kein Array ist? Wer übergibt denn movies? Ist das eine unkontrollierbare Quelle, die alles mögliche liefern kann? Oder ist es höchstens wahrscheinlich, dass es entweder ein Array oder undefined ist? Kann man dem Aufrufer zumuten, ein || [] anzuhängen, um statt undefined ein leeres Array zu liefern? Oder kann das vielleicht in der Funktion stattfinden?

return (movies || []).map(...)

Vielleicht reicht es auch, das undefined so abzufangen, dass kein Fehler geworfen wird, aber auch undefined statt leerem Array als Antwort kommt?

return movies?.map(...)

So kann man auch mit minimalem Aufwand die wahrscheinlichsten Probleme abfangen.

Zu 2: Ist es wahrscheinlich, dass Werte kein String sind?

In einem Template-Literal ist es ja egal, welchen Typ der Wert hat. Notfalls kommt beim automatischen Typecasten zu String 'undefined' oder '[object Object]' raus. Stört (mich) das, wenn jemand Garbage zurückbekommt, wenn Garbage übergeben wird?

Oder wie wäre es mit einem einfachen Typecast?

return movies?.map(movie => doWhatEver(String(movie)));

Ohne Nummer: Wie wahrscheinlich ist es, dass Müll übergeben wird? Vom EVA-Prinzip ausgehend: Steckt diese Funktion im Verarbeitungsteil, sollte man davon ausgehen können, dass der Eingabeteil bereits die gröbsten Probleme erfasst und zurückgewiesen hat. Ansonsten könnte man überlegen, warum das nicht geschehen ist.

Kommen die Daten aus einer Quelle? Warum ist die Fehlererkennung nicht bereits in dem Teil, der die Daten holt?

Generell gesagt: Lohnt sich so eine aufwendige Fehlererkennung überhaupt, oder kann mit Disziplin und Programmstruktur dafür gesorgt werden, dass keine ungültigen Parameter übergeben werden.

Hier auch noch eine dritte Empfehlung für TypeScript. Wenn man sich über die Typen im klaren ist und überall explizite Typeangaben notiert (solange der Typ nicht implizit klar ist), hat man einen Haufen Nachlässigkeitsprobleme gar nicht erst. Zusammen mit einer IDE, die aufgrund der Typinformation, die Typescript erkennt oder auswerten kann, direkt beim Tippen die Probleme erkennt und auch nur gültige Codevervollständigung vorschlägt, ergibt sich ein wertvolles Gespann zur Produktivitätssteigerung.

dedlfix.

[Rolf B]

Hallo dedlfix,

Runtime-Checks macht man normalerweise nur an der Außenfassade, ja.

Und in einer guten IDE sollte es auch funktionieren, den Code mit entsprechenden JSDoc-Kommentaren zu spicken. Sie greift daraus dann die Deklarationen für den Linter ab und kann mit Intellisense oder Warnungen im Editor dienen.

https://jsdoc.app/howto-es2015-classes.html

Rolf

[dedlfix]

Tach!

Vielen Dank für eure Hilfe und Tipps. Ich habe hier jetzt ein Beispiel erstellt: https://codepen.io/ebody/pen/bGoRbPR

Neben RolfBs Tipps solltest du dir vor allem die Frage stellen: Muss die Prüfung und das was sie antwortet überhaupt so penibel und ausführlich sein?

Abgesehen davon ist einiges dabei, das man kürzer schreiben kann.

Array.isArray() und Vergleiche à la typeof x == 'string'oder x > 0 liefern bereits true oder false. Das muss man nicht nochmal auswerten und dann auch nichts anderes als true oder false zurückgeben.

Auch klammerst du zu viel. Array.isArray(val) ist ziemlich eindeutig, das muss man nicht noch extra klammern. Klammern erhöhen die Lesbarkeit und tragen zum Erkennen der Intention bei, auch wenn sie technisch überflüssig sind. Aber sowas zahlt sich eher bei komplexen Ausdrücken aus. Bei einfachen Ausdrücken erhöhen sie nur die Komplexität ohne weiteren Nutzen. Dass bei einem (val.length > 0) ? true : false das > eine höhere Priorität als der ternäre Operator hat, sollte man einerseits wissen, andererseits ist es ziemlich klar, dass nicht nur die 0 vom ternären Operator ausgewertet werden soll.

dedlfix.

[Der Martin]

Hallo,

Dass bei einem (val.length > 0) ? true : false das > eine höhere Priorität als der ternäre Operator hat, sollte man einerseits wissen, andererseits ist es ziemlich klar, dass nicht nur die 0 vom ternären Operator ausgewertet werden soll.

noch dazu liefern auch die Vergleichsoperatoren ihrerseits schon ein true oder false, so dass die Auswertung mit dem ternären Operator und der expliziten Notation von true und false hier unnötig ist und auch wieder nur die Komplexität erhöht und damit die Lesbarkeit verschlechtert.

Womit wir wieder am Anfang wären. 😉

Immer eine Handbreit Wasser unterm Kiel
 Martin

[ebody]

Hallo dedlfix,

vielen Dank für dein Feedback. Habe ich alles angepasst.

Gruß ebody