From fa63b07a69e6b5bf23f2ee84d089f3647a197cb2 Mon Sep 17 00:00:00 2001
From: tobiglaser <76131623+tobiglaser@users.noreply.github.com>
Date: Tue, 2 Aug 2022 23:04:52 +0200
Subject: [PATCH] lots of ReadMe additions about Worksheet 3

---
 README.md | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/README.md b/README.md
index 2ebc977..84a24c1 100644
--- a/README.md
+++ b/README.md
@@ -5,6 +5,49 @@ The Course is about programming concepts rather than a specific language.
 
 ## Changes
 
+### Worksheet 3
+**Background**
+  - Many files were added so that all excercises could be solved, enough funtionality was introduced to allow some level of simulation.
+  - `ComHandler` and `RoverHandler` were implemented as Singletons as they are not members of `ControlModel`.  
+    They could be implemented differently. With `getInstance()` as the interface to get a reference to the `ComHandler` instance this will not break the implementation.
+  - `ComState`, an enumeration class, was introduced to handle the States of `ComHandler` in a simpler, more defined way.
+  - `ObjectFileHandler`
+    - The vectors are passed by reference.
+    - `dynamic_pointer_cast` is used for polymorphism using `shared_ptr`.
+    - *!Currently there is a problem because the OFH has to include e.g. `Gear.h` to instanciate commands,*
+      *but `Gear.h` us implemented by the user.*
+      It is fine for a file from `src/` to include from `include/`, but not the other way around.
+      "Solved" by including with manual path `#include "../src/Gear.h"` rather than relying on CMake.
+
+**CommandType**
+  - Yields a `std::shared_ptr<ICommand>` rather than a `Command`.
+  - `Element` was extended with a constructor that takes an existing `shared_ptr`, rather then creating a new one.
+    This was done because it would be unnecessary to extract the raw `Command` out of the returned `shared_ptr` to `Command`,
+    only to pass it to `Element` where it is wrapped into a `shared_ptr` again.
+
+**ControlModelRegistry**
+  - The registry holds `weak_ptr`s rather than `shared_ptr`s because it should not own the `listeners`. `Weak pointers` are locked and checked for validity when they are used.
+  - Earlier `std::erase` was used to remove `listeners` from the `registry`, but this doesn't work for `weak_ptr`, so a for-loop with an `iterator` is used.
+  - To notify the `listeners` a *range based for-loop* is used.  
+    Here one can see how code complexity can be reduced by using modern features of Cpp.
+
+**ControlModel**
+  - The instance variable of the singleton pattern was moved to inside the `getInstance()` function reducing complexity.
+  - To register the `ControlModel` to the `ComHandler` the raw pointer had to be used.
+    This should be ok as the lifetime of the singleton instance should last until the program is terminated. The raw pointer was wrapped into an `optional` in `ComHandler` which might be unnecessary because it could also be `nullptr`. It is more expressive tough.
+  - `commandPerformed()` receives the `shared_ptr ICommand` wrapped into an `optional` as well.
+    Again not necessary, but clearly states that the `command` is `optional`.
+    Because of the `dynamic_cast` to `Command` the `optional` is redundant.
+  - `start()` gathers every `shared_ptr<ICommand>` saved in `CommandList`
+    A simple `for-loop` has to be used, after writing `ControlModelRegistry` with `for-ranged` and `iterator based for-loops` this feels wrong and unsafe.
+    Maybe a good opportunity to write an own `iterator` for `CommandList`?
+  - The `getters` return references to private data of `ControlModel`, likely `const` should be used to protect them from the harsh world outside `ControlModel`.
+  - Since no `Rover` could be selected `getSelectedRoverId()` returns an `optional string`.
+**Test**
+  - The test was expanded also show adding `Commands` from the `CommandTypes` as well as reading from and writing to a file.
+  - Because `ObjectFileHandler` was created, another test was added for for that.
+
+
 ### Worksheet 2
 **Background**
   - `commandlib.h` was introduced as a stand in for `hsrt.mec.controldeveloper.core.com.command`. For simplicity everything is defined in one header.
-- 
GitLab