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

[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