From 8f910277aee526a6496424b72b1b07af5a23fe47 Mon Sep 17 00:00:00 2001
From: Joel Tio <joeltiojinhon@gmail.com>
Date: Mon, 8 Feb 2021 13:40:41 +0800
Subject: [PATCH 1/5] feat: add Getter optic with tests

---
 sim/lib/core/CMakeLists.txt             |  1 +
 sim/lib/core/include/core/optics/lens.h | 66 +++++++++++++++++++++++++
 sim/lib/core/src/optics/lens.test.cpp   | 36 ++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 sim/lib/core/include/core/optics/lens.h
 create mode 100644 sim/lib/core/src/optics/lens.test.cpp

diff --git a/sim/lib/core/CMakeLists.txt b/sim/lib/core/CMakeLists.txt
index 06ebe8a..be140b7 100644
--- a/sim/lib/core/CMakeLists.txt
+++ b/sim/lib/core/CMakeLists.txt
@@ -38,6 +38,7 @@ add_library(core-test
     src/ics/util/composable.test.cpp
     src/ics/util/setIntersection.test.cpp
     src/ics/util/typeMap.test.cpp
+    src/optics/lens.test.cpp
 )
 target_link_libraries(core-test
     PRIVATE core
diff --git a/sim/lib/core/include/core/optics/lens.h b/sim/lib/core/include/core/optics/lens.h
new file mode 100644
index 0000000..31e97a0
--- /dev/null
+++ b/sim/lib/core/include/core/optics/lens.h
@@ -0,0 +1,66 @@
+#ifndef BENTOBOX_LENS_H
+#define BENTOBOX_LENS_H
+
+#include <functional>
+
+namespace optics {
+
+template <class Return, class... Args>
+class Getter {
+   private:
+    const std::function<Return(Args...)> getter;
+
+   public:
+    // A getter cannot be constructed without a getter function
+    Getter() = delete;
+    explicit Getter(std::function<Return(Args...)>&& getter) : getter(getter) {}
+
+    Return operator()(Args&&... args) {
+        return std::invoke(getter, std::forward<Args>(args)...);
+    }
+
+    template <class NewReturn>
+    Getter<NewReturn, Args...> compose(
+        std::function<NewReturn(Return)>&& composeGetter) {
+        // Make a copy of the getter so that if the current Getter is destroyed
+        // The returned getter is still usable
+        std::function<Return(Args...)> getterCopy(getter);
+        return Getter<NewReturn, Args...>(
+            [getterCopy, composeGetter](Args... args) {
+                return std::invoke(
+                    composeGetter,
+                    std::invoke(getterCopy, std::forward<Args>(args)...));
+            });
+    }
+
+    // Oddly, C++ does not recognise lambdas when std::function is used here.
+    // So, C++20 requirements are used.
+    // The attempted definition of a std::function is:
+    //    template<class...NewArgs>
+    //    Getter<Return, NewArgs...>
+    //    precompose(std::function<std::tuple_element_t<0,
+    //    std::tuple<Args...>>(NewArgs...)> precomposeGetter) {
+    template <class... NewArgs, class Fn>
+        // Require that the current getter only needs one argument
+        requires(sizeof...(Args) == 1) &&
+        // Require that the type Fn is something invocable with the NewArgs and
+        // returns the first argument
+        std::is_invocable_r_v<
+            std::tuple_element_t<0, std::tuple<Args...>>, Fn,
+            NewArgs...> Getter<Return,
+                               NewArgs...> precompose(Fn&& precomposeGetter) {
+        // Make a copy of the getter so that if the current Getter is destroyed
+        // The returned getter is still usable
+        std::function<Return(Args...)> getterCopy(getter);
+        return Getter<Return, NewArgs...>([getterCopy,
+                                           precomposeGetter](NewArgs... args) {
+            return std::invoke(
+                getterCopy,
+                std::invoke(precomposeGetter, std::forward<NewArgs>(args)...));
+        });
+    }
+};
+
+}  // namespace optics
+
+#endif  // BENTOBOX_LENS_H
diff --git a/sim/lib/core/src/optics/lens.test.cpp b/sim/lib/core/src/optics/lens.test.cpp
new file mode 100644
index 0000000..3112408
--- /dev/null
+++ b/sim/lib/core/src/optics/lens.test.cpp
@@ -0,0 +1,36 @@
+#include <gtest/gtest.h>
+#include <core/optics/lens.h>
+
+#define TEST_SUITE Lenstest
+
+using namespace optics;
+
+TEST(TEST_SUITE, GetterFromLambda) {
+    // From rvalue
+    {
+        auto getter = Getter<int, int, char>([](int j, int k) { return 'a'; });
+        ASSERT_EQ(getter(5, 7), 'a');
+    }
+
+    // From lvalue
+    {
+        auto lambdaGetter = [](int j, int k) { return j + k; };
+        auto getter = Getter<int, int, int>(lambdaGetter);
+        ASSERT_EQ(getter(9, 10), 19);
+    }
+}
+
+TEST(TEST_SUITE, ComposeGetters) {
+    auto getter = Getter<int, int, int>([](int j, int k) { return j + k; });
+    auto composedGetter = getter.compose<int>([](int h) { return h + 20; });
+
+    ASSERT_EQ(composedGetter(1, 2), 23);
+}
+
+TEST(TEST_SUITE, PrecomposeGetters) {
+    auto getter = Getter<int, int>([](int j) { return j * 10; });
+    auto precomposedGetter =
+        getter.precompose<int>([](int h) -> int { return h + 20; });
+
+    ASSERT_EQ(precomposedGetter(1), 210);
+}

From 21d6a6325dcbbc308c516b39c219625ce45f78a0 Mon Sep 17 00:00:00 2001
From: Joel Tio <joeltiojinhon@gmail.com>
Date: Tue, 9 Feb 2021 15:54:46 +0800
Subject: [PATCH 2/5] feat: add lens with tests

---
 sim/lib/core/include/core/optics/lens.h | 128 +++++++++++++++++++++---
 sim/lib/core/src/optics/lens.test.cpp   |  93 +++++++++++++++--
 2 files changed, 199 insertions(+), 22 deletions(-)

diff --git a/sim/lib/core/include/core/optics/lens.h b/sim/lib/core/include/core/optics/lens.h
index 31e97a0..df93228 100644
--- a/sim/lib/core/include/core/optics/lens.h
+++ b/sim/lib/core/include/core/optics/lens.h
@@ -2,31 +2,75 @@
 #define BENTOBOX_LENS_H
 
 #include <functional>
+#include <type_traits>
+
+namespace {
+    template<class ...Args>
+    struct is_same_args {
+        // This static constexpr function compares the given type arguments and
+        // ensures that they are the same as Args
+        template<size_t N = 0, class ...OtherArgs>
+        static constexpr bool _as() {
+            // constexpr is required here because N is a non-type template
+            // argument
+            if constexpr (N == sizeof...(OtherArgs)) {
+                return true;
+            } else if (std::is_same_v<
+                std::remove_reference_t<
+                    std::tuple_element_t<N, std::tuple<OtherArgs...>>>,
+                std::remove_reference_t<
+                    std::tuple_element_t<N, std::tuple<Args...>>>
+            >) {
+                return _as<N+1, OtherArgs...>();
+            } else {
+                return false;
+            }
+        }
+
+        // Alias to the recursive comparison constexpr function
+        template<class ...OtherArgs>
+        static constexpr bool as() {
+            return _as<0, OtherArgs...>();
+        }
+    };
+}
 
 namespace optics {
 
+    // Note on std::move vs static_cast<std::remove_reference_t<T>&&>
+    // Later in the code, you will see that I have used static_cast instead of
+    // std::move. The reason is that we need to cast to an rvalue reference
+    // because that's the type of the function's parameter.
+    // std::move is not used because it is semantically incorrect, though it
+    // does the same thing.
+
 template <class Return, class... Args>
 class Getter {
-   private:
-    const std::function<Return(Args...)> getter;
+   protected:
 
    public:
+    const std::function<Return(std::remove_reference_t<Args>&&...)> getter;
     // A getter cannot be constructed without a getter function
     Getter() = delete;
-    explicit Getter(std::function<Return(Args...)>&& getter) : getter(getter) {}
+    explicit Getter(std::function<Return(Args...)>&& getter)
+        : getter(std::move(getter)) {}
+    explicit Getter(const std::function<Return(Args...)>& getter)
+        : getter(getter) {}
 
-    Return operator()(Args&&... args) {
-        return std::invoke(getter, std::forward<Args>(args)...);
+    template<class ...RArgs>
+    requires (is_same_args<Args...>::template as<RArgs...>())
+    Return get(RArgs&&... args) const {
+        return getter(static_cast<std::remove_reference_t<RArgs>>(args)...);
     }
 
     template <class NewReturn>
     Getter<NewReturn, Args...> compose(
-        std::function<NewReturn(Return)>&& composeGetter) {
+        std::function<NewReturn(Return)>&& composeGetter) const {
         // Make a copy of the getter so that if the current Getter is destroyed
         // The returned getter is still usable
         std::function<Return(Args...)> getterCopy(getter);
         return Getter<NewReturn, Args...>(
-            [getterCopy, composeGetter](Args... args) {
+            [getterCopy, composeGetter](Args&&... args) {
                 return std::invoke(
                     composeGetter,
                     std::invoke(getterCopy, std::forward<Args>(args)...));
@@ -46,18 +90,70 @@ class Getter {
         // Require that the type Fn is something invocable with the NewArgs and
         // returns the first argument
         std::is_invocable_r_v<
-            std::tuple_element_t<0, std::tuple<Args...>>, Fn,
-            NewArgs...> Getter<Return,
-                               NewArgs...> precompose(Fn&& precomposeGetter) {
+            std::tuple_element_t<0, std::tuple<Args...>>,
+            Fn,
+            NewArgs...>
+        Getter<Return, NewArgs...> precompose(Fn&& precomposeGetter) const {
         // Make a copy of the getter so that if the current Getter is destroyed
         // The returned getter is still usable
         std::function<Return(Args...)> getterCopy(getter);
-        return Getter<Return, NewArgs...>([getterCopy,
-                                           precomposeGetter](NewArgs... args) {
-            return std::invoke(
-                getterCopy,
-                std::invoke(precomposeGetter, std::forward<NewArgs>(args)...));
-        });
+        return Getter<Return, NewArgs...>(
+            [getterCopy, precomposeGetter](NewArgs&&... args) {
+                return std::invoke(getterCopy,
+                                   std::invoke(precomposeGetter,
+                                               std::forward<NewArgs>(args)...));
+            });
+    }
+};
+
+template <class Return, class... Args>
+class Lens {
+   private:
+    typedef std::function<void(Return&, Return&&)> SetterFn;
+    // When writing the getter as a lambda, it is important to specify the
+    // arguments as rvalue references and the return type as an lvalue
+    // reference.
+    typedef std::function<Return&(Args&&...)> GetterFn;
+
+   public:
+    const SetterFn setter;
+    const GetterFn getter;
+    // A lens cannot be constructed without any functions
+    Lens() = delete;
+    Lens(GetterFn&& getter, SetterFn&& setter)
+        : getter(std::move(getter)), setter(std::move(setter)) {}
+    Lens(const GetterFn& getter, const SetterFn& setter)
+        : getter(getter), setter(setter) {}
+
+    template<class ...RArgs>
+    requires (is_same_args<Args...>::template as<RArgs...>())
+    Return& get(RArgs&&... args) const {
+        return getter(static_cast<std::remove_reference_t<RArgs>&&>(args)...);
+    }
+
+    template<class RReturn, class ...RArgs>
+    // Ensure that the arguments are the correct type
+    requires (is_same_args<Args...>::template as<RArgs...>())
+    // Ensure that RReturn is the correct type
+        && std::is_same_v<std::remove_reference_t<RReturn>, Return>
+    void set(RReturn&& val, RArgs&&... args) const {
+        auto& ref = get(std::forward<RArgs>(args)...);
+        this->setter(ref, static_cast<std::remove_reference_t<RReturn>&&>(val));
+    }
+
+    template <class NewReturn>
+    Lens<NewReturn, Args...> compose(
+        const Lens<NewReturn, Return>& otherLens) const {
+        std::function<NewReturn&(Args&&...)> get = [
+            getter = this->getter,
+            otherGetter = otherLens.getter
+        ]<class ...PArgs>(PArgs&&... args) -> NewReturn& {
+              return otherGetter(
+                  std::forward<Return>(getter(std::forward<PArgs>(args)...)));
+        };
+
+        std::function<void(NewReturn&, NewReturn&&)> set = otherLens.setter;
+        return Lens<NewReturn, Args...>(get, set);
     }
 };
 
diff --git a/sim/lib/core/src/optics/lens.test.cpp b/sim/lib/core/src/optics/lens.test.cpp
index 3112408..1d8ad8f 100644
--- a/sim/lib/core/src/optics/lens.test.cpp
+++ b/sim/lib/core/src/optics/lens.test.cpp
@@ -8,15 +8,19 @@ using namespace optics;
 TEST(TEST_SUITE, GetterFromLambda) {
     // From rvalue
     {
-        auto getter = Getter<int, int, char>([](int j, int k) { return 'a'; });
-        ASSERT_EQ(getter(5, 7), 'a');
+        auto getter = Getter<char, int, int>([](int j, int k) { return 'a'; });
+        ASSERT_EQ(getter.get(5, 7), 'a');
     }
 
     // From lvalue
     {
-        auto lambdaGetter = [](int j, int k) { return j + k; };
+        auto lambdaGetter = []<class T, class U>(T&& j, U&& k) {
+            return j + k;
+        };
         auto getter = Getter<int, int, int>(lambdaGetter);
-        ASSERT_EQ(getter(9, 10), 19);
+        // This is to test that get can accept an lvalue
+        auto val = 9;
+        ASSERT_EQ(getter.get(val, 10), 19);
     }
 }
 
@@ -24,7 +28,7 @@ TEST(TEST_SUITE, ComposeGetters) {
     auto getter = Getter<int, int, int>([](int j, int k) { return j + k; });
     auto composedGetter = getter.compose<int>([](int h) { return h + 20; });
 
-    ASSERT_EQ(composedGetter(1, 2), 23);
+    ASSERT_EQ(composedGetter.get(1, 2), 23);
 }
 
 TEST(TEST_SUITE, PrecomposeGetters) {
@@ -32,5 +36,82 @@ TEST(TEST_SUITE, PrecomposeGetters) {
     auto precomposedGetter =
         getter.precompose<int>([](int h) -> int { return h + 20; });
 
-    ASSERT_EQ(precomposedGetter(1), 210);
+    ASSERT_EQ(precomposedGetter.get(1), 210);
+}
+
+TEST(TEST_SUITE, LensFromLambda) {
+    // From rvalue
+    {
+        int storedVal = 3;
+        auto lens =
+            Lens<int, int>([](int&& j) -> int& { return j; },
+                           [](int& j, int newVal) -> void { j = newVal - 10; });
+        ASSERT_EQ(lens.get(3), 3);
+        lens.set(14, storedVal);
+        ASSERT_EQ(storedVal, 4);
+    }
+
+    // From lvalue
+    {
+        int storedVal = 3;
+        auto getter = [](int&& j, int&& k) -> int& { return j; };
+        auto setter = [](int& j, int newVal) { j = newVal - 10; };
+        auto lens = Lens<int, int, int>(getter, setter);
+        // This is to test that get can accept an lvalue
+        ASSERT_EQ(lens.get(storedVal, 10), 3);
+        lens.set(14, storedVal, 5);
+        ASSERT_EQ(storedVal, 4);
+    }
+}
+
+struct B {
+    int height = 0;
+};
+
+struct A {
+    B b;
+};
+
+TEST(TEST_SUITE, ComposeLens) {
+    auto storedVal = A{B{10}};
+    auto getter = [](A&& a) -> B& { return a.b; };
+    auto setter = [](B& b, B&& newB) { b = newB; };
+    auto lens = Lens<B, A>(getter, setter);
+
+    ASSERT_EQ(lens.get(storedVal).height, 10);
+
+    auto getterComp = [](B&& b) -> int& { return b.height; };
+    auto setterComp = [](int& height, int&& newHeight) { height = newHeight; };
+    auto lensComp = Lens<int, B>(getterComp, setterComp);
+
+    auto composedLens = lens.compose(lensComp);
+    ASSERT_EQ(composedLens.get(storedVal), 10);
+    composedLens.set(20, storedVal);
+    ASSERT_EQ(composedLens.get(storedVal), 20);
+}
+
+TEST(TEST_SUITE, CompositionCopiesLenses) {
+    // Create the lenses
+    auto getter = [](A&& a) -> B& { return a.b; };
+    auto setter = [](B& b, B&& newB) { b = newB; };
+    auto lensAPtr = new Lens<B, A>(getter, setter);
+
+    auto getterComp = [](B&& b) -> int& { return b.height; };
+    auto setterComp = [](int& height, int&& newHeight) { height = newHeight; };
+    auto lensBPtr = new Lens<int, B>(getterComp, setterComp);
+
+    auto composedLens = lensAPtr->compose(*lensBPtr);
+
+    // Delete the lenses
+    delete lensAPtr;
+    lensAPtr = nullptr;
+    delete lensBPtr;
+    lensBPtr = nullptr;
+
+    // Use the composed lens
+    auto storedVal = A{B{10}};
+    ASSERT_EQ(composedLens.get(storedVal), 10);
+    auto j = 20;
+    composedLens.set(j, storedVal);
+    ASSERT_EQ(composedLens.get(storedVal), 20);
 }

From 2d69bf0f7e42ee630f1b32778ad3fcb0b49e83de Mon Sep 17 00:00:00 2001
From: Joel Tio <joeltiojinhon@gmail.com>
Date: Wed, 10 Feb 2021 13:35:26 +0800
Subject: [PATCH 3/5] fix: clang format cannot recognise lens.h

---
 sim/lib/core/include/core/optics/{lens.h => lens.hpp} | 0
 sim/lib/core/src/optics/lens.test.cpp                 | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename sim/lib/core/include/core/optics/{lens.h => lens.hpp} (100%)

diff --git a/sim/lib/core/include/core/optics/lens.h b/sim/lib/core/include/core/optics/lens.hpp
similarity index 100%
rename from sim/lib/core/include/core/optics/lens.h
rename to sim/lib/core/include/core/optics/lens.hpp
diff --git a/sim/lib/core/src/optics/lens.test.cpp b/sim/lib/core/src/optics/lens.test.cpp
index 1d8ad8f..db6f055 100644
--- a/sim/lib/core/src/optics/lens.test.cpp
+++ b/sim/lib/core/src/optics/lens.test.cpp
@@ -1,5 +1,5 @@
 #include <gtest/gtest.h>
-#include <core/optics/lens.h>
+#include <core/optics/lens.hpp>
 
 #define TEST_SUITE Lenstest
 

From 3e431dfa70a3be550d69585011782f3961ab50fd Mon Sep 17 00:00:00 2001
From: Joel Tio <joeltiojinhon@gmail.com>
Date: Wed, 10 Feb 2021 18:16:23 +0800
Subject: [PATCH 4/5] refactor: lint lens.hpp

---
 sim/lib/core/include/core/optics/lens.hpp | 119 +++++++++++-----------
 1 file changed, 59 insertions(+), 60 deletions(-)

diff --git a/sim/lib/core/include/core/optics/lens.hpp b/sim/lib/core/include/core/optics/lens.hpp
index df93228..3c1ea27 100644
--- a/sim/lib/core/include/core/optics/lens.hpp
+++ b/sim/lib/core/include/core/optics/lens.hpp
@@ -1,53 +1,51 @@
-#ifndef BENTOBOX_LENS_H
-#define BENTOBOX_LENS_H
+#ifndef BENTOBOX_LENS_HPP
+#define BENTOBOX_LENS_HPP
 
 #include <functional>
 #include <type_traits>
 
 namespace {
-    template<class ...Args>
-    struct is_same_args {
-        // This static constexpr function compares the given type arguments and
-        // ensures that they are the same as Args
-        template<size_t N = 0, class ...OtherArgs>
-        static constexpr bool _as() {
-            // constexpr is required here because N is a non-type template
-            // argument
-            if constexpr (N == sizeof...(OtherArgs)) {
-                return true;
-            } else if (std::is_same_v<
-                std::remove_reference_t<
-                    std::tuple_element_t<N, std::tuple<OtherArgs...>>>,
-                std::remove_reference_t<
-                    std::tuple_element_t<N, std::tuple<Args...>>>
-            >) {
-                return _as<N+1, OtherArgs...>();
-            } else {
-                return false;
-            }
+template <class... Args>
+struct is_same_args {
+    // This static constexpr function compares the given type arguments and
+    // ensures that they are the same as Args
+    template <size_t N = 0, class... OtherArgs>
+    static constexpr bool _as() {
+        // constexpr is required here because N is a non-type template
+        // argument
+        if constexpr (N == sizeof...(OtherArgs)) {
+            return true;
+        } else if (std::is_same_v<
+                       std::remove_reference_t<
+                           std::tuple_element_t<N, std::tuple<OtherArgs...>>>,
+                       std::remove_reference_t<
+                           std::tuple_element_t<N, std::tuple<Args...>>>>) {
+            return _as<N + 1, OtherArgs...>();
+        } else {
+            return false;
         }
+    }
 
-        // Alias to the recursive comparison constexpr function
-        template<class ...OtherArgs>
-        static constexpr bool as() {
-            return _as<0, OtherArgs...>();
-        }
-    };
-}
+    // Alias to the recursive comparison constexpr function
+    template <class... OtherArgs>
+    static constexpr bool as() {
+        return _as<0, OtherArgs...>();
+    }
+};
+}  // namespace
 
 namespace optics {
 
-    // Note on std::move vs static_cast<std::remove_reference_t<T>&&>
-    // Later in the code, you will see that I have used static_cast instead of
-    // std::move. The reason is that we need to cast to an rvalue reference
-    // because that's the type of the function's parameter.
-    // std::move is not used because it is semantically incorrect, though it
-    // does the same thing.
+// Note on std::move vs static_cast<std::remove_reference_t<T>&&>
+// Later in the code, you will see that I have used static_cast instead of
+// std::move. The reason is that we need to cast to an rvalue reference
+// because that's the type of the function's parameter.
+// std::move is not used because it is semantically incorrect, though it
+// does the same thing.
 
 template <class Return, class... Args>
 class Getter {
    protected:
-
    public:
     const std::function<Return(std::remove_reference_t<Args>&&...)> getter;
     // A getter cannot be constructed without a getter function
@@ -57,9 +55,9 @@ class Getter {
     explicit Getter(const std::function<Return(Args...)>& getter)
         : getter(getter) {}
 
-    template<class ...RArgs>
-    requires (is_same_args<Args...>::template as<RArgs...>())
-    Return get(RArgs&&... args) const {
+    template <class... RArgs>
+    requires(is_same_args<Args...>::template as<RArgs...>()) Return
+        get(RArgs&&... args) const {
         return getter(static_cast<std::remove_reference_t<RArgs>>(args)...);
     }
 
@@ -90,10 +88,10 @@ class Getter {
         // Require that the type Fn is something invocable with the NewArgs and
         // returns the first argument
         std::is_invocable_r_v<
-            std::tuple_element_t<0, std::tuple<Args...>>,
-            Fn,
-            NewArgs...>
-        Getter<Return, NewArgs...> precompose(Fn&& precomposeGetter) const {
+            std::tuple_element_t<0, std::tuple<Args...>>, Fn,
+            NewArgs...> Getter<Return,
+                               NewArgs...> precompose(Fn&& precomposeGetter)
+            const {
         // Make a copy of the getter so that if the current Getter is destroyed
         // The returned getter is still usable
         std::function<Return(Args...)> getterCopy(getter);
@@ -125,18 +123,18 @@ class Lens {
     Lens(const GetterFn& getter, const SetterFn& setter)
         : getter(getter), setter(setter) {}
 
-    template<class ...RArgs>
-    requires (is_same_args<Args...>::template as<RArgs...>())
-    Return& get(RArgs&&... args) const {
+    template <class... RArgs>
+    requires(is_same_args<Args...>::template as<RArgs...>()) Return& get(
+        RArgs&&... args) const {
         return getter(static_cast<std::remove_reference_t<RArgs>&&>(args)...);
     }
 
-    template<class RReturn, class ...RArgs>
-    // Ensure that the arguments are the correct type
-    requires (is_same_args<Args...>::template as<RArgs...>())
-    // Ensure that RReturn is the correct type
-        && std::is_same_v<std::remove_reference_t<RReturn>, Return>
-    void set(RReturn&& val, RArgs&&... args) const {
+    template <class RReturn, class... RArgs>
+        // Ensure that the arguments are the correct type
+        requires(is_same_args<Args...>::template as<RArgs...>())
+        // Ensure that RReturn is the correct type
+        && std::is_same_v<std::remove_reference_t<RReturn>, Return> void set(
+               RReturn&& val, RArgs&&... args) const {
         auto& ref = get(std::forward<RArgs>(args)...);
         this->setter(ref, static_cast<std::remove_reference_t<RReturn>&&>(val));
     }
@@ -144,19 +142,20 @@ class Lens {
     template <class NewReturn>
     Lens<NewReturn, Args...> compose(
         const Lens<NewReturn, Return>& otherLens) const {
-        std::function<NewReturn&(Args&&...)> get = [
-            getter = this->getter,
-            otherGetter = otherLens.getter
-        ]<class ...PArgs>(PArgs&&... args) -> NewReturn& {
-              return otherGetter(
-                  std::forward<Return>(getter(std::forward<PArgs>(args)...)));
+        std::function<NewReturn&(Args && ...)> get =
+            [
+                getter = this->getter, otherGetter = otherLens.getter
+            ]<class... PArgs>(PArgs && ... args)
+                ->NewReturn& {
+            return otherGetter(
+                std::forward<Return>(getter(std::forward<PArgs>(args)...)));
         };
 
-        std::function<void(NewReturn&, NewReturn&&)> set = otherLens.setter;
+        std::function<void(NewReturn&, NewReturn &&)> set = otherLens.setter;
         return Lens<NewReturn, Args...>(get, set);
     }
 };
 
 }  // namespace optics
 
-#endif  // BENTOBOX_LENS_H
+#endif  // BENTOBOX_LENS_HPP

From 756829275275d0f4328e4da58ddb07eacb264b39 Mon Sep 17 00:00:00 2001
From: Joel Tio <joeltiojinhon@gmail.com>
Date: Wed, 10 Feb 2021 18:35:02 +0800
Subject: [PATCH 5/5] feat: add lens precomposition

---
 sim/lib/core/include/core/optics/lens.hpp | 24 ++++++++++++++++++++---
 sim/lib/core/src/optics/lens.test.cpp     | 18 +++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/sim/lib/core/include/core/optics/lens.hpp b/sim/lib/core/include/core/optics/lens.hpp
index 3c1ea27..4d37695 100644
--- a/sim/lib/core/include/core/optics/lens.hpp
+++ b/sim/lib/core/include/core/optics/lens.hpp
@@ -148,11 +148,29 @@ class Lens {
             ]<class... PArgs>(PArgs && ... args)
                 ->NewReturn& {
             return otherGetter(
-                std::forward<Return>(getter(std::forward<PArgs>(args)...)));
+                static_cast<Return&&>(getter(std::forward<PArgs>(args)...)));
         };
 
-        std::function<void(NewReturn&, NewReturn &&)> set = otherLens.setter;
-        return Lens<NewReturn, Args...>(get, set);
+        return Lens<NewReturn, Args...>(get, otherLens.setter);
+    }
+
+    template <class FirstArg, class... NewArgs>
+        requires(sizeof...(Args) == 1) &&
+        std::is_same_v<
+            std::tuple_element_t<0, std::tuple<Args...>>,
+            FirstArg> Lens<Return, NewArgs...> precompose(const Lens<FirstArg,
+                                                                     NewArgs...>
+                                                              otherLens) const {
+        std::function<Return&(NewArgs && ...)> get =
+            [
+                getter = this->getter, otherGetter = otherLens.getter
+            ]<class... PArgs>(PArgs && ... args)
+                ->Return& {
+            return getter(static_cast<FirstArg&&>(
+                otherGetter(std::forward<PArgs>(args)...)));
+        };
+
+        return Lens<Return, NewArgs...>(get, this->setter);
     }
 };
 
diff --git a/sim/lib/core/src/optics/lens.test.cpp b/sim/lib/core/src/optics/lens.test.cpp
index db6f055..ac90bb8 100644
--- a/sim/lib/core/src/optics/lens.test.cpp
+++ b/sim/lib/core/src/optics/lens.test.cpp
@@ -90,6 +90,24 @@ TEST(TEST_SUITE, ComposeLens) {
     ASSERT_EQ(composedLens.get(storedVal), 20);
 }
 
+TEST(TEST_SUITE, PrecomposeLens) {
+    auto storedVal = A{B{10}};
+    auto getter = [](A&& a) -> B& { return a.b; };
+    auto setter = [](B& b, B&& newB) { b = newB; };
+    auto lens = Lens<B, A>(getter, setter);
+
+    ASSERT_EQ(lens.get(storedVal).height, 10);
+
+    auto getterComp = [](B&& b) -> int& { return b.height; };
+    auto setterComp = [](int& height, int&& newHeight) { height = newHeight; };
+    auto lensComp = Lens<int, B>(getterComp, setterComp);
+
+    auto composedLens = lensComp.precompose(lens);
+    ASSERT_EQ(composedLens.get(storedVal), 10);
+    composedLens.set(20, storedVal);
+    ASSERT_EQ(composedLens.get(storedVal), 20);
+}
+
 TEST(TEST_SUITE, CompositionCopiesLenses) {
     // Create the lenses
     auto getter = [](A&& a) -> B& { return a.b; };