TU Wien:Software Engineering und Projektmanagement PR (Biffl)/SEPM Einzelphase Feedback

Hier wird das SEPM Einzelphase Feedback gesammelt, damit man als Studierender weiß, worauf man achten muss.

Und damit man als SEPM Tutor sich leichter tut beim reproduzierbaren beurteilen, da es seltsam ist wenn zwei fast identische Abgaben sehr verschiedenes Feedback bekommen.

Hinweise: Eine ähnliche Liste gibt es ebenfalls aus dem Jahr 2021S: Feedback 2021S.

User Stories

User Story 1 - Neue Pferde anlegen

• -1 Pferde anlegen: Name darf nicht nur aus Leerzeichen bestehen.
• -2 Es kann kein Pferd ohne Owner gespeichert werden, es wird zwar ein Request gesendet mit dem Statuscode 201, das Pferd wird jedoch nicht in der Datenbank gespeichert.

User Story 2 - Bestehende Pferde bearbeiten

• -3 keine vorgegebenen Werte beim bearbeiten, somit praktisch ein erstellen und kein bearbeiten mehr

User Story 3 - Pferde löschen

• -1 Löschen aus Detailansicht nicht möglich
• -2 Löschen eines Kindes löscht Verweise der Eltern für alle Kinder

• -1 Löschen aus Suchansicht nicht möglich

User Story 4 - Für ein Pferd Eltern zuweisen

• -4 Es ist möglich, 2 männliche Pferde als Elternteile anzulegen. Es ist möglich, 2 weibliche Pferde als Elternteile anzulegen.

User Story 6 - Nach Pferden suchen

• -3 Suche ist limitiert auf 10 Ergebnisse, das macht den Eindruck dass die Ergebnisse falsch sind.
• -2 Wird nach einem Owner gesucht und dieser dann wieder herausgelöscht, so ist die Suche anschließend unbrauchbar. (Anmerkung von mir: Löschen des owners von der Suche, nicht aus der DB)
• -2 Eingabe von Sonderzeichen führt zu fehlerhafem Verhalten [?name=foo in description]
• -2 Suche nach Besitzer mit Leertaste darin funktioniert nicht
• -2 Suche Man kann den Owner nicht mit Teilstrings suchen

User Story 7 - Neue Besitzer/innen anlegen

• -1 Email ist nicht optional (lässt sich über Frontend nur mit Email anlegen)

User Story 9 - Stammbaum eine Pferdes ansehen

• -4 Baum ist initial nicht ausgeklappt bzw. werden keine Gererationen vorausgewählt.
• -6 Baumansicht: Es wird nicht bei der angegeben Anzahl an Generationen abgeschnitten.
• -9 Generation auf 3 gesetzt, aber insgesamt werden 4 angezeigt & Gereration auf eins beschränkt, dennoch werden Elternpferde angezeigt

Tech Stories

Tech Story 10 - Backend Logfiles

• -2 Oft kein Loggen von privaten Methoden, manchmal auch bei public Methoden.
• -9 Loglevel in application.yml nicht der angabe entsprechend angepasst vereinzelt wurde vergessen private methoden mit trace zu loggen Es wurde an zwei stellen in den Tests System.out.println verwendet statt ordentlich zu loggen
• -6 Fehlt an einer Stelle; nicht konstant - einmal mit debug dann wieder trace; System.out.println 2 mal
• -3 at.ac.tuwien.sepm wird nicht auf DEBUG geloggt
• -3 Loglevel Config ist falsch(DEBUG statt TRACE gefordert) (Anmerkung von mir: In der Angabe steht "zumindest" DEBUG also mMn kein Fehler TRACE zu verwenden)
• -3 Loglevel der Dependencies sollte INFO ist und für die Applikation selbst DEBUG
• -2 Logging Es wurde in getOwner im HorseMapper nicht geloggt Im HorseJdbcDao wurde mapRow nicht geloggt (hier wäre log.trace zu verwenden gewesen) Es wird auch nicht ownerMapForSingleId in HorseServiceImpl geloggt
• -3 Log Konfiguration nicht richtig gesetzt. Nur Root auf Info gesetzt.

Tech Story 11 - Programm in Englisch geschrieben und dokumentiert

• -1 Im OwnerService interface fehlt bei der create Methode JavaDoc für die ConflictException
• -1 @throws NotFoundException fehlt in HorseService interface.

Tech Story 12 - Stundenliste

• -1 Führung der Stundenliste: Gesamtsumme der Stunden in der Stundenliste stimmt nicht.

Tech Story 13 - Kernfunktionalitäten getestet

• -2 Tests nicht immer sinnvoll
• -3 Nur 2 Testmethoden für Endpoints und kein schreibender Test für Rest Layer
• -2 Tests Bei Endpointtests sollte man ein Result von mockMvc zurückbekommen, welches man dann über Assertions mit einem ""expected value"" vergleichen sollte.
• Bei negativ Tests ist es gut, wenn man assertThrows verwendet (Kein Punkteabzug, nur ein Tipp)

Tech Story 14 - Sauberen Git Workflow

• -3 Aussagekräftige Commitmessage: Commitmessages sind nicht aussagekräftig und sinnvoll.
• -3 Log files, Datenbank, IntelliJ Files, und einige andere Files in Git Historie. Teilweise sind Commits sehr groß und behandeln verschiedene Tasks/Komponenten obwohl sich diese hätten aufteilen lassen (und dabei trotzdem in sich abgeschlossen bleiben).
• -2 keine commit messages bei den letzten paar commits
• -3 es wurde einmal .idea und .iml commited

Tech Story 15 - Alle Eingaben validiert

• -6 Das Geschlecht von Elternteilen kann so verändert werden, dass ein Kind zwei Väter oder zwei Mütter hat. Eltern können so editiert werden, dass sie jünger sind als ihre Kinder (Validierung wird nur eine Richtung durchgeführt). Somit ist es weiters möglich "Kreise" zu erzeugen, indem man anschließend beim Elternteil das Kind als Vater oder Mutter einträgt. Zu langer Name (ab 256 Zeichen) eines Pferdes oder Owners führt zu Datenbankfehler.
• -1 Eltern können so editiert werden, dass sie jünger sind als ihre Kinder (Validierung wird nur eine Richtung durchgeführt). Somit ist es weiters möglich "Kreise" zu erzeugen, indem man anschließend beim Elternteil das Kind als Vater oder Mutter einträgt.
• -6 horse und owner name wird nicht auf länge validiert Durch zu langen namen beim erstellen von Pferden und ownern kommt es zu Datenbankfehlern
• -3 Conflict errors werden erst geworfen nachdem horse bereits in db geupdated worden ist
• -6 Geburtstag eines Pferdes kann in der Zukunft liegen; Überlange Strings führen zu Datenbankfehler
• -3 Geburtsdatum des Pferdes kann in der Zukunft liegen. Geburtsdatum der Eltern kann in die Zukunft gestellt werden, sodass jünger als Kind. Mutter kann jünger als das Kind sein.
• -3 email is nicht optional und es kann ein freitext als email eingegeben werden. updaten des geschlechts der eltern möglich.
• -2 Validierung Kann Geschlecht von Vater auf weiblich setzen (dasselbe geht für die Mutter) Die Email sollte nach dem pattern ""a@b.com"" validiert werden

Tech Story 16 - Sinnvolles Code-Design

• -2 4 Unused Imports in Abgabe
• -1 Unused Files ValidationErrorRestDto, ConflictErrorRestDto in Abgabe
• -3 checkstyle im frontend schlägt fehl mit 4 errors und 6 problems

Tech Story 18 - Übersichtliches Frontend

• -3 Wird bei der Erstellung kein Geburtsdatum eingetragen, so wird das Pferd trotzdem angelegt - mit dem heutigen Tag als Geburtsdatum. Für den Benutzer ist dies erst nach Erstellung ersichtlich. Eingabefelder in Detailansicht sind keine passenden UI-Elemente.
• -1 Horse details button in create ansicht ist wenig sinnvoll und führt zu horse ansicht eines unvollständigen pferdes
• -1 Navigation zu Eltern aus Detailsicht nicht klar ersichtlich.

Tech Story 19 - Frontend sinnvoll bedienbar

• -1 Es ist nicht ersichtlich, dass das Datumsfeld das heutige Datum annimmt, wenn der Benutzer nichts eingibt.
• -1 Wird bei der Erstellung kein Geburtsdatum eingetragen, so wird das Pferd trotzdem angelegt - mit dem heutigen Tag als Geburtsdatum. Für den Benutzer ist dies erst nach Erstellung ersichtlich.
• -1 Wenn bei der Suche in ein Textfeld geschrieben wird und danach Enter gedrückt wird, werden wieder alle Pferde geladen. Der Text im Suchfeld wird ignoriert.

Tech Story 20 - Frontend über Ereignisse informiert

• -2 Suche limitiert auf 10 Ergebnisse ohne das dem Benutzer mitzuteilen.
• -2 500er Fehler werden im frontend nicht angezeigt wodurch der user nicht weiß was passiert ist
• -2 Fehlermeldung bei keiner Eingabe beim Stammbaum
• -4 Eingabe von Vater/Mutter erst möglich, wenn Datum ausgewählt (create); Default-page bei start-up is blank (erst durch click auf horses auf richtiger Seite)
• -2 Zu langer name von Pferd zeigt Datenbankfehlermeldung.
• -2 Wenn bei der Erstellung eines Pferdes kein Datum gesetzt wird, wird es zwar nicht gespeichert, aber es kommt die Meldung “Horse Testy successfully created”. Dem Nutzer ist damit nicht klar, was passiert.
• -2 Fehlermeldung "Could not create owner: Validation of owner for create failed" nicht aussagekräftig
• -2 Fehlermeldung Wenn Geburtstag von Kind so bearbeitet wird, dass es vor dem eines oder mehrere Elternteile ist, dann bekommt man keine Fehlermeldung im Frontend(wird aber gecheckt im Backend).

Tech Story 23 - Programmarchitektur

• -5 Die UI Schicht baut die HttpParams zusammen, obwohl das im Service passieren sollte. Im Validator wird überprüft ob der Owner existiert, aber nicht indem er angefragt und eine NotFoundException gefangen wird sondern über eine eigene existsById() Methode.

Tech Story 25 - Umgang mit Ressourcen

• -4 Auf der Startseite/Listenseite wird beim initialen Laden zwei Mal ein Request mit der ganzen Pferdeliste geschickt. Einer von diesen ist unnötig und verursacht nur unnötig Traffic!
• -5 Zur Validierung eines Pferdes, werden unnötigerweise alle Owner und alle Pferde aus der Datenbank geladen.
• -4 Kein Debounce bei der Suche
• -10 Suche wird im Frontend gemacht; keine speziellen DTOs zu Übertragung
• -11 insgesamt für folgende drei Fehler:
  • Baum wird geladen bevor man überhaupt weiß wieviele Generation gefragt sind.
  • Auf der Hauptseite wird zwei mal GET /horses gecalled.
  • Vollständiges Eltern DTO in Detailansicht/Hauptseite eines Pferdes geladen. (Name/ID wäre ausreichend).
• -5 mother & father in der liste, father, mother, owner in the create bei der father und mother auswahl (Very Confusing sentence, aber bei den Suggestions sind alle Eltern + Owner zu sehen)

Tech Story 26 - Fehlerbehandlung

• -9 Es werden nie ConflictExceptions geworfen sondern ausschließlich ValidationExceptions, auch bei Konflikten. Im Validator wird eine NotFoundException gefangen, mit level ERROR geloggt und dann die Exception der weitergegebenen ValidationException nicht mitgegeben. Nicht bei allen subscribes gibt es eine Fehlerbehandlung.
• -3 Bei der Methode HorseServiceImpl.horseMapForIds wird eine NotFoundException gewrappt, aber nicht an die neue FatalException weitergegeben! Somit geht wichtige Debug-Info verloren!
• -9 Exceptions (e.g. NotFoundExceptions) werden unnötigerweise auf Log warn geloggt Erstellt man wie beschrieben einen "Kreis" (e.g. Pferd A ist Vater von Pferd B und Pferd B ist Mutter von Pferd A), so führt dies zu einer Endlosschleife im Backend und einem 500er wenn die Detailseite eines betroffenen Pferdes aufgerufen wird. Werden Eltern so editiert, dass ein Kind zwei Väter oder zwei Mütter hat, so lässt sich das Kind nur editieren, wenn man diesen Konflikt wieder auflöst. (Anmerkung von mir: Für diesen Fehler hab ich insgesamt - 15 Punkt bekommen.)
• -1 An Orten wo Status Code 409 erwartet wird wird stattdessen 422 geworfen (zB Vater jünger als geupdatetes Pferd)
• -3 Manchmal wird Exception gefangen und neue geworfen, ohne der neuen Exception die alte mitzugeben.
• -2 Zwei subscribe Blöcke werden nicht auf error überprüft
• -8 insgesamt für folgende zwei Fehler:
  • Exception wird in HorseServiceImpl gefangen und neue geworfen, ohne der neuen Exception die alte mitzugeben: Hierbei geht der Stack Trace und damit wertvolle Debug-Information verloren!
  • Programm stürzt ab wenn der Name des Pferdes zu lang ist. Ebenso beim Owner.

Tech Story 27 - REST Konformität

• -6 Immer 422er wo 409er gefragt wäre, zB 500er statt 404er.
• -2 Aufbau von URIs: URIs dürfen keine Verben enthalten, z.B. ist /owners/create verboten.
• -2 /horses/search nicht REST-konform (bei /owners ist es richtig gemacht worden) (Anmerkung von mir: Ich habe es ident zum Owner gemacht...)
• -1 ValidationExc statt Conflict bei Pferd jünger als Vater
• -1 Wenn beim Bearbeiten eines Pferdes versucht wird, das Datum vor dem Geburtsdatum eines Elternteils zu setzen, wird der Statuscode 422 statt 409 zurückgeliefert

Tech Story 28 - Daten in Datenbank persistieren

• -3 Wenig Testdaten, decken kaum Funktionalität ab

Tech Story 29 - Datenmodell

• -5 Keine Referenz auf den Owner. Fehlende UNIQUE constraint auf die Owner Email.
• -2 Email nicht unique.
• -8 keine fremdschlüssel verwendet. email nicht unique.

Tech Story 31 - Trennung von Produktiv- und Testdatenbank

• -5 keine Testisolation gegeben (zB durch transaktionen)