Doku

2-Application/src/main/java/link/LinkIdGenerator.java

@@ -1,8 +0,0 @@
-package link;
-
-import category.CategoryId;
-
-public interface LinkIdGenerator {
-
-	LinkId generateId();
-}

Documentation/Main.java

@@ -0,0 +1,36 @@
+
+public class Main {
+
+    public static void main(String[] args) {
+        System.out.print("success");
+    }
+
+    abstract public class Subcommand {
+
+        public String executeSubcommand(String[] args);
+        final public HashMap<String, Function<String[], String>> commands =
+            new HashMap<>();
+
+        abstract public String getSubcommand();
+
+        abstract public String getUsage();
+
+        public String executeSubcommand(String[] args) {
+            try {
+                commandExsits(args[0]);
+                return commands.get(args[0]).apply(args);
+            }
+            catch (IndexOutOfBoundsException e) {
+                throw new CliError("Missing a value! " +
+                                   getUsage());
+            }
+        }
+
+        private void commandExsits(String command) {
+            if (commands.get(command) == null) {
+                throw new CliError("Subcommand does not exist! " +
+                                   getUsage());
+            }
+        }
+    }
+}

Documentation/programmEntwurf.org

@@ -208,11 +208,24 @@ und möglicher Lösungsweg des Negativ-Beispiels (inkl. UML)]
      :PROPERTIES:
      :CUSTOM_ID: positiv-beispiel
      :END:
+
+     [[./uml/CategoryName.png]]
+
+Die Klasse CategoryName repräsentiert den Namen einer Kategorie und legt dabei fest, welche Werte dieser annehmen kann.
+
 **** Negativ-Beispiel
      :PROPERTIES:
      :CUSTOM_ID: negativ-beispiel
      :END:
 
+     [[./uml/CommandHandler.png]]
+     [[./uml/CommandHandlerClasses.png]]
+
+Die Klasse CommandHandler wird aufgerufen und leitet die CLI Parameter an die einzelnen SubCommand-Klassen weiter.
+Da die SubCommand-Klassen jedoch für ihre Konstruktoren, die Adapter benötigen, die Adapter wiederum die Usecases benötigen usw., wird der gesamte Baum an benötigten Klassen im Konstruktor der CommandHandler Klasse aufgebaut.
+Dies ist jedoch nicht ihre Responsibility.
+**Lösung**: Der CommandHandler bekommt die SubCommand als Konstruktorparameter eingreicht und das Erstellen der restlichen Klassen wird von einer dedizierten Klasse durchgeführt. (UML nicht skizziert, weil es schneller ist den Code zu fixen und dann das UML zu generieren, als das UML per Hand zu machen.)
+
 *** Analyse Open-Closed-Principle (OCP)
     :PROPERTIES:
     :CUSTOM_ID: analyse-open-closed-principle-ocp
@@ -227,11 +240,69 @@ lösen (inkl. UML)?]
      :PROPERTIES:
      :CUSTOM_ID: positiv-beispiel-1
      :END:
+
+[[./uml/TagMatcher.png]]
+
+Die Klasse TagMatcher bietet das Interface (in diesem Fall als abstrakte Klasse) für alle möglichen Test, ob einer URL ein gewisser Tag zugeordnet werden kann.
+
+Statt eines Switchstatements wie hier gezeigt:
+#+begin_src java
+switch(url):
+    case githubRegexPatter.matches(url):
+      tags.add(new GitHubTag())
+    case someOtherMatcher.matches(url):
+      tags.add(new GitHubTag())
+#+end_src
+
+Wird für einen Link von allen TagMatchern zur Laufzeit geprüft, ob dieser matcht.
+So können beispielsweise benutzerdefinierte Matcher verwendet werden (siehe Klasse CustomTagMatcher) und wenn ein neuer TagMatcher mit besonderer Implementation hinzufügt wird muss er nur die abstrakte Klasse erweitern und zur Liste die während der Dependecy-Injection-Phase gebaut wird hinzugefügt werden.
+In den Domain, Application und Adapter Schichten muss hierfür kein Code angepasst werden.
+
+#+begin_src java
+public Set<Tag> getTagsFor(LinkUrl url) {
+		Set<Tag> result = new HashSet<>();
+		tagMatcherRepository.getTagMatchers().forEach(
+                     tagMatcher -> tagMatcher.ifMatches(url).
+                        addTo(result));
+		return result;
+	}
+
+
+#+end_src
+
 **** Negativ-Beispiel
      :PROPERTIES:
      :CUSTOM_ID: negativ-beispiel-1
      :END:
 
+[[./uml/Exeptions.png]]
+
+Die Exeptions der Anwendung erfüllen nicht wirklich das OCP.
+So gibt es einen Try-Catch-Block um die gesamte Anwendung.
+
+#+begin_src java
+try {
+	commandHandler.executeCommand(args);
+}
+catch (PersistenceError persistenceError) {
+	System.out.println("There was a Error with loading
+                       or saving the persistence data.");
+	System.out.println(persistenceError.getMessage());
+}
+catch (RuntimeException runtimeException) {
+	System.out.println(runtimeException.getMessage());
+}
+#+end_src
+
+Fügt man eine neue Exeption hinzu muss man zwar keinen Catch-Block hinzufügen um die Lauffähigkeit zu erhalten, es wäre jedoch für die Benutzerfreundlichkeit deutlich besser (vgl. extra Nachricht bei PersistenceError) wenn man es täte.
+Damit hierfür dann nicht für jede Exeption ein Catch-Block hinzugefügt werden muss sollten die Exeptions semantisch gruppiert werden und gemeinsame Elternklassen haben.
+So könnte man die Elternklasse Domainerror einfügen, für Exeptions, die innerhalb der Domäne liegen und keinen Programmfehler sondern eine falsche Nutzerhandlung bedeuten.
+Darunter würden Exeption wie CategoryAlreadyExists fallen.
+Sind diese definiert kann man leichter neue Exeptions hinzufügen ohne die Catchblöcke anpassen zu müssen oder dem Nutzer schlechte/inkonsistente Ausgaben zu geben.
+
+UML nicht vorhanden, da es schneller wäre den Fix einzubauen und das UML zu generieren als das UML von Hand zu bauen.
+
+
 *** Analyse Liskov-Substitution- (LSP), Interface-Segreggation- (ISP), Dependency-Inversion-Principle (DIP)
     :PROPERTIES:
     :CUSTOM_ID: analyse-liskov-substitution--lsp-interface-segreggation--isp-dependency-inversion-principle-dip
@@ -240,19 +311,20 @@ lösen (inkl. UML)?]
 LSP oder ISP oder DIP); jeweils UML der Klasse und Begründung, warum man
 hier das Prinzip erfüllt/nicht erfüllt wird]
 
-[Anm.: es darf nur ein Prinzip ausgewählt werden; es darf NICHT z.B. ein
+
-positives Beispiel für LSP und ein negatives Beispiel für ISP genommen
+**Dependency-Inversion-Principle**
-werden]
 
 **** Positiv-Beispiel
-     :PROPERTIES:
+     [[./uml/CategoryIdGenerator.png]]
-     :CUSTOM_ID: positiv-beispiel-2
-     :END:
-**** Negativ-Beispiel
-     :PROPERTIES:
-     :CUSTOM_ID: negativ-beispiel-2
-     :END:
 
+     Beim der Klasse CategoryIdGenerator wird das DIP erfüllt.
+     Die Klasse CategoryUseCase ist nicht anhängig von einer konkreten Implementation eines IdGenerators wie dem RandomCategoryIdGenerator sondern von dem Interface CategoryIdGenerator.
+     Dies ist auch besonders für Test praktisch, da man dann nicht mit Zufallszahlen umgehen muss.
+
+**** Negativ-Beispiel
+     [[./uml/LinkUseCase.png]]
+     Beim der Klasse LinkUseCase wird das DIP nicht erfüllt.
+     Die Klasse LinkUseCase ist anhängig von konkreten Implementation eines IdGenerators, dem RandomLinkIdGenerator.
 * Kapitel 4: Weitere Prinzipien
   :PROPERTIES:
   :CUSTOM_ID: kapitel-4-weitere-prinzipien
@@ -291,10 +363,62 @@ Begründung, warum die Kohäsion hoch ist]
 aufgelöst wurde; Code-Beispiele (vorher/nachher); begründen und
 Auswirkung beschreiben]
 
-* 
+Das Interface SubCommand definiert einen CLI SubCommand, der wiederum einzelne Funktionen hat.
-  :PROPERTIES:
+Diese einzelnen Funktionen werden in einer Map gespeichert, welche den Namen auf die Methode mappt.
-  :CUSTOM_ID: section-1
+Die Logik hierfür war zunächst in jeder Implementation von SubCommand gleich implementiert.
-  :END:
+
+#+begin_src java
+public class CategoryCommands extends Subcommand {
+
+	final private CategoryCliAdapter categoryCliAdapter;
+	final private HashMap<String, Function<String[], String>> commands =
+        new HashMap<>();
+
+	@Override
+	public String executeSubcommand(String[] args) {
+		return commands.get(args[0]).apply(args);
+	}
+}
+#+end_src
+
+Indem das Interface SubCommand zu einer abstrakten Klasse umgebaut wurde, wurde die Logik an eine zentrale Stelle verschoben und zusätzlich gleich das benötigte Errorhandling eingebaut.
+
+
+#+begin_src java
+abstract public class Subcommand {
+
+	public String executeSubcommand(String[] args);
+	final public HashMap<String, Function<String[], String>> commands =
+        new HashMap<>();
+
+	abstract public String getSubcommand();
+
+	abstract public String getUsage();
+
+	public String executeSubcommand(String[] args) {
+		try {
+			commandExsits(args[0]);
+			return commands.get(args[0]).apply(args);
+		}
+		catch (IndexOutOfBoundsException e) {
+			throw new CliError("Missing a value! " +
+                               getUsage());
+		}
+	}
+
+	private void commandExsits(String command) {
+		if (commands.get(command) == null) {
+			throw new CliError("Subcommand does not exist! " +
+                               getUsage());
+		}
+	}
+}
+#+end_src
+
+#+RESULTS:
+
+Die angebenen Änderungen sind im Commit [[https://tea.filefighter.de/qvalentin/LinkDitch/commit/78730bc69f][78730bc69f]] sichtbar. Da die neuen Implementation des Interfaces im selben Commit hinzugefügt wurden ist, wurden diese direkt ohne den duplizierten Code committet.
+
 * Kapitel 5: *Unit Tests*
   :PROPERTIES:
   :CUSTOM_ID: kapitel-5-unit-tests
@@ -305,17 +429,34 @@ Auswirkung beschreiben]
     :END:
 [Nennung von 10 Unit-Tests und Beschreibung, was getestet wird]
 
-| Unit Test      | Beschreibung |
+
-| Klasse#Methode |              |
+
-|                |              |
+1. CategoryIdTest#ConstructorWorks
-|                |              |
+   Stellt sicher, dass eine CategoryId mit einem int erstellt werden kann.
-|                |              |
+   Da ich vorher noch nie Java-Records verwendet hatte, war es ganz gut mit einem kleinen Test deren Funktionalität zu überprüfen.
-|                |              |
+   Gerne hätte ich auch einen Test zur Unveränderbarkeit von Records gemacht, beim Versuch einen solchen zu schreiben wurde allerdings klar, dass es keine Methoden gibt die Veränderungen bewirken und Records somit die Anforderungen erfüllen (auch wenn es nicht testbar ist).
-|                |              |
+2. CategoryId#equalsWorks
-|                |              |
+   Stellt sicher, dass zwei CategoryIds die mit dem selben int erstellt wurden durch die equals Methode als gleich angesehen werden.
-|                |              |
+   Erneut eine Überprüfung, dass Records sich wie erwartet verhalten.
-|                |              |
+3. CategoryNameTest#getNameWorks
-|                |              |
+   Stellt sicher, dass der Getter für Name den erwarteten Wert zurück liefert.
+4. CategoryNameTest#constructorThrowsNull,constructorThrowsBlank,constructorThrowsEmpty,constructorThrowsTooShort
+   Stellen sicher, dass die Regeln die für den Namen einer Category definiert sind auch korrekt überprüft werden und im Fehlerfall eine entsprechende Exeption geschmissen wird.
+5. CategoryEntityTest#categoryConversionWorks
+   Stellt sicher, dass bei der Konvertierung zwischen Category und CategorEntity durch die Funktionen toCategory und den Konstruktor.
+   Die Komposition der beiden Funktionen sollte die Identitätsfunktion ergeben.
+6. LinkEntityTest#toCSVString
+   Stellt sicher, dass das serialisieren eines Objektes zu einem CSV String das erwartete Ergebnis liefert.
+7. LinkEntityTest#fromCSVString
+   Stellt sicher, dass das de-serialisieren eines CSV Strings das erwartete Ergebnis liefert.
+8. CategoryCommandsTest#addCommandWorks
+   Stellt sicher, dass beim Aufruf der Methode executeSubcommand mit dem String "add" und einem CategoryNamen die korrekte Methode des Adapters aufgerufen wird und die Parameter korrekt weitergereicht werden und eine passende Erfolgsmeldung geliefert wird.
+9. GenericCSVDAOTest#addWorks
+   Stellt sicher, dass nach dem Hinzufügen eines CategorEntitys dieses auch wieder gefunden werden kann.
+10. GitHubTagMatcherTest#gettingDescriptionWorks
+    Überprüft die Interaktion mit der GitHub Repository API indem für ein konkretes Repository der Wert abgefragt wird.
+
+
 
 *** ATRIP: Automatic
     :PROPERTIES:
@@ -323,6 +464,50 @@ Auswirkung beschreiben]
     :END:
 [Begründung/Erläuterung, wie ‘Automatic' realisiert wurde]
 
+Automatic wurde durch die einfache Ausführbarkeit realisiert.
+So muss nur ein Befehl ausgeführt werden um die Test zu starten.
+  #+begin_src
+mvn clean test
+  #+end_src
+
+Alternativ genügen auch wenige Clicks bzw. Shortcuts in der IDE um die Test auszuführen und detailreiches Feedback über ihren Erfolg zu erhalten.
+
+[[./img/test-ide.png]]
+
+Die Test laufen ohne Eingaben und liefern immer ein Ergebnis, dass eindeutig Erfolg oder Misserfolg bezeugt.
+
+Damit man die Test nicht immer nur lokal ausführen muss werden sie auch bei jedem einchecken des Codes in das entfernte Versionskontrollrepository auf dem Server ausgeführt.
+Dies ist mit Drone CI realisiert und liefert in der Weboberfläche der Versionskontrolle schnelles Feedback.
+
+#+begin_src yaml
+---
+kind: pipeline
+type: docker
+name: Tests Coverage
+
+steps:
+  - name: Run Tests With Coverage
+    image: maven:3.8-openjdk-17-slim
+    environment:
+      SONAR_LOGIN:
+        from_secret: SONAR_TOKEN
+    commands:
+      - mvn clean verify sonar:sonar -s ./settings.xml
+trigger:
+  branch:
+    include:
+      - master
+  trigger:
+    event:
+      - push
+#+end_src
+
+[[./img/gitea-test.png]]
+
+Mit Drone CI bekommt man dann gleich auch einen guten Überblick, wie oft die Test fehlschlagen und wie lange das Testen braucht.
+
+[[./img/drone-test.png]]
+
 *** ATRIP: Thorough
     :PROPERTIES:
     :CUSTOM_ID: atrip-thorough
@@ -331,6 +516,16 @@ Auswirkung beschreiben]
 Code-Beispiel, Analyse und Begründung, was professionell/nicht
 professionell ist]
 
+**** Positives Beispiel
+Beim CategoryNameTest werden (bis auf generierten Code) sämtliche Methoden getestet und sämtliche Sonderfälle für die Eingaben in einzelnen Test geprüft (constructorThrowsNull,constructorThrowsBlank,constructorThrowsEmpty,constructorThrowsTooShort)
+
+[[./img/categoryNameTest.png]]
+
+**** Negatives Beispiel
+Beim CategorEntityTest werden bis auf die beiden Koversationsmethoden Richtung Category keine Methoden getestet, obwohl beispielsweise die Konvertierung nach CSV leicht einen Fehler enthalten könnte, der Probleme verursachen würde (z.B. vergessenes toString).
+
+[[./img/CategoryEntityTest.png]]
+
 *** ATRIP: Professional
     :PROPERTIES:
     :CUSTOM_ID: atrip-professional
@@ -339,12 +534,106 @@ professionell ist]
 Code-Beispiel, Analyse und Begründung, was professionell/nicht
 professionell ist]
 
+**** Positives Beispiel
+Der Test GenericCSVDAOTest verwendet eine Datei. Damit dies sauber abläuft wird eine temporäre Datei verwendet.
+#+begin_src java
+File file = File.createTempFile("test","link-ditch");
+#+end_src
+
+Vor und nach jedem Test wird die Datei gesäubert, damit die Test alle im gleichen Zustand starten.
+
+#+begin_src java
+@BeforeEach
+public void beforeEach() throws IOException {
+
+	if (file.exists()) {
+		file.delete();
+	}
+	file.createNewFile();
+
+	this.sut = new GenericCSVDAO<>(file, CategoryEntity::new);
+}
+
+@AfterEach
+void afterEach() {
+	file.delete();
+}
+#+end_src
+
+Damit die Tests lesbarer sind wird das recht aufwendige erzeugen eines CategoryEntitys und hinzufügen dessen in eine private Hilfsfunktion ausgelagert.
+
+#+begin_src java
+private CategoryEntity addDummyEntity(String categoryName, int id) {
+	var entityToAdd = new CategoryEntity(categoryName, id);
+	sut.add(entityToAdd);
+	return entityToAdd;
+}
+#+end_src
+
+Somit liest sich der removeAllWorks Test deutlich besser und duplizierter Code wird vermieden, was wiederum Fehler vermeidet.
+#+begin_src java
+@Test
+public void removeAllWorks() throws IOException {
+	addDummyEntity("categoryName1", 101);
+	addDummyEntity("categoryName2", 102);
+	assertEquals(2, sut.getALl().size());
+	sut.removeAll();
+
+	assertEquals(0, sut.getALl().size());
+}
+#+end_src
+
+**** Negatives Beispiel
+
+Der Test addCommandWorks ist nicht sehr professionell.
+Es werden schlechte Variablennamen wie category1 und category2 verwendet.
+Die Verwendung des ArgumentCaptors macht den Code schlecht lesbar.
+Immerhin werden Variablen verwendet und nicht die Strings an allen Stellen hardgecoded.
+
+#+begin_src java
+@Test
+void addCommandWorks() {
+	var url = "http://tea.filefighter.de";
+	var username = "mario";
+	var category1 = "funStuff";
+	var category2 = "workStuff";
+
+	ArgumentCaptor<String> captureUrl = ArgumentCaptor.forClass(String.class);
+	ArgumentCaptor<String> captureUsername = ArgumentCaptor.forClass(String.class);
+	ArgumentCaptor<Set<String>> captureCategories = ArgumentCaptor.forClass(Set.class);
+
+	doNothing()
+			.when(mockAdapter)
+			.addLink(captureUrl.capture(), captureCategories.capture(), captureUsername.capture());
+
+	var sut = new LinkCommands(mockAdapter);
+	var returnValue = sut.executeSubcommand(new String[]{"add", url, username, category1, category2});
+
+	assertEquals("Added the new Link", returnValue);
+
+	assertEquals(url, captureUrl.getValue());
+	assertEquals(username, captureUsername.getValue());
+	assertEquals(Set.of(category1, category2), captureCategories.getValue());
+}
+#+end_src
+
+*** Zusatz: ATRIP: Repeatable
+
+Der Commit [[https://tea.filefighter.de/qvalentin/LinkDitch/commit/d1fdad7cf9][d1fdad7cf9]] zeigt den Fix für einen Test der nicht repeatable war, weil bei Sets die Reihenfolge der Elemente nicht eindeutig ist und somit zufällig.
+
 *** Code Coverage
     :PROPERTIES:
     :CUSTOM_ID: code-coverage
     :END:
 [Code Coverage im Projekt analysieren und begründen]
 
+Die Coverage ist mit etwa 25 % deutlich zu niedrig.
+Da es sich bei dem Projekt jedoch um Code handelt, der niemals wirklich produktiv eingesetzt werden wird und der nicht langfristig weiterentwickelt wird, ist dies verkraftbar.
+Es wurden hauptsächlich die komplizierteren Stellen getestet, wie beispielsweise die CSV-Persistierung und die Interaktion mit der Github-APi.
+Bei diesen Stellen wurden grade genug Test geschrieben, um sicherzustellen, dass die Grundfunktion korrekt ist.
+Teilweise wurde während des Entwicklungsprozesses gemerkt, dass es besser gewesen wäre, manche Stellen zu testen.
+Für Fehler die beim manuellen Testen der Anwendung aufgefallen waren wurden teilweise extra Tests geschrieben.
+
 *** Fakes und Mocks
     :PROPERTIES:
     :CUSTOM_ID: fakes-und-mocks
@@ -352,6 +641,63 @@ professionell ist]
 [Analyse und Begründung des Einsatzes von 2 Fake/Mock-Objekten;
 zusätzlich jeweils UML Diagramm der Klasse]
 
+
+1. CategoryCommandsTest:
+   Beim CategoryCommandsTest wurde die Klasse CategoryCommands erstellt, doch anstatt ein Objekt der Klasse CategoryCliAdapter beim Erstellen zu übergeben wurde ein Mock übergeben.
+
+   [[./uml/CategoryCommandsTest.png]]
+
+    Dieses Mock wird dann aufgerufen und die Parameter des Aufrufs werden überprüft.
+    Zusätzlich wird definiert, welche Rückgabewerte das Mock liefern soll.
+    #+begin_src java
+    class CategoryCommandsTest {
+
+        CategoryCliAdapter mockAdapter = mock(CategoryCliAdapter.class);
+
+        @Test
+        void addCommandWorks() {
+            var categoryName = "funStuff";
+            ArgumentCaptor<String> valueCapture = ArgumentCaptor.forClass(String.class);
+            doNothing().when(mockAdapter).addCategory(valueCapture.capture());
+            var sut = new CategoryCommands(mockAdapter);
+
+            var returnValue = sut.executeSubcommand(new String[]{"add", categoryName});
+
+            assertEquals(categoryName, valueCapture.getValue());
+            assertEquals("Added the new category", returnValue);
+        }
+
+        @Test
+        void getCommandWorks() {
+            var sut = new CategoryCommands(mockAdapter);
+            when(mockAdapter.getCategories()).thenReturn(Set.of("funStuff", "workStuff"));
+            var returnValue = sut.executeSubcommand(new String[]{"get"});
+
+            var expected =
+                    "Available Categories:" + System.lineSeparator() + "funStuff" + System.lineSeparator() + "workStuff";
+
+            var expectedDifferentOrder =
+                    "Available Categories:" + System.lineSeparator() + "workStuff" + System.lineSeparator() + "funStuff";
+            assertTrue(expected.equals(returnValue) || expectedDifferentOrder.equals(returnValue));
+        }
+    }
+    #+end_src
+
+    Das Mock ist hier besonders nützlich, da wir uns an der Grenze zwischen zwei Schichten befinden.
+    Für das Erstellen des CategoryCliAdapters wird eine Usecase benötigt, welcher wiederum Instanzen aus der Domäne benötigt, welche wiederum bestimmte Instanzen benötigen.
+    Indem wir stattdessen ein Mock erstellen werden quasi alle anderen Schichten weg abstrahiert.
+    Dies ist auch empfehlenswert, da wir beim aktuellen Unittest ja nur die Funktionalität der aktuellen Klasse testen wollen.
+    Deshalb definieren wir durch das Mock, wie sich der Rest der Anwendung verhalten sollte und können uns auf unsere aktuell Klasse konzentrieren und sind unabhängig von eventuellen Bugs in anderen Bereichen oder fehlenden Implementationen.
+
+2. LinkCommandsTest:
+
+   [[./uml/LinkCommandsTest.png]]
+   Beim LinkCommandsTest wurde die Klasse LinkCommands erstellt, doch anstatt ein Objekt der Klasse LinkCliAdapter beim Erstellen zu übergeben wird ein Mock übergeben.
+
+   Auch hier befinden wir uns an der Grenze von zwei Schichten.
+
+
+
 * Kapitel 6: Domain Driven Design
   :PROPERTIES:
   :CUSTOM_ID: kapitel-6-domain-driven-design
@@ -471,7 +817,7 @@ sowie UML vorher/nachher liefern; jeweils auf die Commits verweisen]
 Absprache auch andere) jeweils sinnvoll einsetzen, begründen und
 UML-Diagramm]/
 
-*** Entwurfsmuster: [Name]
+*** Entwurfsmuster: Dekorator
     :PROPERTIES:
     :CUSTOM_ID: entwurfsmuster-name
     :END: