cell refactoring (#5)

src/cell/CMakeLists.txt

@@ -1,6 +1,18 @@
 project(cell)
 
-# TODO: Integrate clang-tidy into the build
+set(CMAKE_CXX_CLANG_TIDY clang-tidy-17 "-config-file=${CMAKE_SOURCE_DIR}/.clang-tidy")
+add_compile_options(-Wall 
+                    -Wextra
+                    -pedantic-errors
+                    -Wnon-virtual-dtor
+                    -Werror
+                    -Wconversion
+                    -Wdouble-promotion
+                    -Wnull-dereference
+                    -Wcast-align
+                    -Wuninitialized
+                    -Wsuggest-override
+                    -Wzero-as-null-pointer-constant)
 
 add_library(${PROJECT_NAME}
     include/${PROJECT_NAME}/cell.h

src/cell/include/cell/cell.h

@@ -15,18 +15,18 @@ namespace Cell {
 
     template <typename T>
     class ICell {
-        public:
-            virtual ~ICell() = default;
-            [[nodiscard]] virtual auto read() const -> std::optional<T> = 0;
-            virtual auto await(Callback<T> callback) -> void = 0;
-            virtual auto block() -> T = 0;
+    public:
+        virtual auto await(Callback<T> callback) -> void = 0;
+        [[nodiscard]] virtual auto read() const -> std::optional<T> = 0;
+        [[nodiscard]] virtual auto block() const -> T = 0;
 
 
-            ICell() = default;
-            ICell(ICell&&) = delete;
-            ICell(const ICell&) = delete;
+        virtual ~ICell() = default;
+        ICell() = default;
+        ICell(ICell&&) = delete;
+        ICell(const ICell&) = delete;
 
-            auto operator=(const ICell&) -> ICell& = delete;
-            auto operator=(ICell&&) -> ICell& = delete;
+        auto operator=(const ICell&) -> ICell& = delete;
+        auto operator=(ICell&&) -> ICell& = delete;
     };
 }

src/cell/include/cell/tracking_once_cell.h

@@ -16,41 +16,40 @@ namespace Cell {
     /// to track can only be specified once
     template <typename T>
     class TrackingOnceCell : public ICell<T> {
-        public:
-            auto read() const -> std::optional<T> override;
-
-            // await takes a callback function and calls it with the value
-            // of the cell being tracked when it is available, if no cell 
-            // is being tracked, the callback is added to a list of callbacks
-            // and added once we have a tracking cell
-            auto await(Callback<T> callback) -> void override;
-
-            // track sets the cell to track, if a cell is already being tracked
-            // the function returns false, otherwise it returns true indicating a successful track
-            // attempt
-            auto track(std::shared_ptr<ICell<T>> new_cell) -> bool;
-
-            // block sleeps the current thread until the value of the cell is available
-            // it then returns the value of the cell, note that this is different from await
-            // as await registers a continuation, while block is a blocking operation
-            auto block() -> T override;
-
-        private:
-            // A note on callbacks:
-            // we need to maintain a set of callbacks for the TrackingCell
-            // as we may not know what cell we are tracking until much later, hence we need to
-            // maintain a set of subscribers to fill once we have a cell to track
-            std::optional<std::shared_ptr<ICell<T>>> cell;
-            std::vector<Callback<T>> callbacks;
-            std::condition_variable_any cell_filled;
-
-            mutable std::shared_mutex mutex;
+    public:
+        [[nodiscard]] auto read() const -> std::optional<T> override;
+
+        // await takes a callback function and calls it with the value
+        // of the cell being tracked when it is available, if no cell 
+        // is being tracked, the callback is added to a list of callbacks
+        // and added once we have a tracking cell
+        auto await(Callback<T> callback) -> void override;
+
+        // track sets the cell to track, if a cell is already being tracked
+        // the function returns false, otherwise it returns true indicating a successful track
+        // attempt
+        auto track(std::shared_ptr<ICell<T>> new_cell) -> bool;
+
+        // block sleeps the current thread until the value of the cell is available
+        // it then returns the value of the cell, note that this is different from await
+        // as await registers a continuation, while block is a blocking operation
+        [[nodiscard]] auto block() const -> T override;
+
+    private:
+        // A note on callbacks:
+        // we need to maintain a set of callbacks for the TrackingCell
+        // as we may not know what cell we are tracking until much later, hence we need to
+        // maintain a set of subscribers to fill once we have a cell to track
+        std::optional<std::shared_ptr<ICell<T>>> cell;
+        std::vector<Callback<T>> callbacks;
+
+        mutable std::condition_variable_any cell_filled;
+        mutable std::shared_mutex mutex;
     };
 }
 
 
 
-// Implementation
 
 
 template <typename T>
@@ -102,7 +101,7 @@ auto Cell::TrackingOnceCell<T>::track(std::shared_ptr<ICell<T>> new_cell) -> boo
 
 
 template <typename T>
-auto Cell::TrackingOnceCell<T>::block() -> T {
+auto Cell::TrackingOnceCell<T>::block() const -> T {
     std::shared_lock lock(mutex);
 
     if (!this->cell.has_value()) { cell_filled.wait(lock); }

src/cell/include/cell/when_all_cell.h

@@ -14,35 +14,35 @@ namespace Cell {
     // and resolves when all of the cells it is tracking resolves
     template <typename T>
     class WhenAllCell : public ICell<std::vector<T>> {
+    public:
+        WhenAllCell(Scheduler::IScheduler& scheduler, std::vector<std::shared_ptr<ICell<T>>> cells);
+
+        [[nodiscard]] auto read() const -> std::optional<std::vector<T>> override;
+        [[nodiscard]] auto block() const -> std::vector<T> override;
+        auto await(Callback<std::vector<T>> callback) -> void override;
+
+    private:
+        struct WhenAllExecutionContext {
         public:
-            WhenAllCell(Scheduler::IScheduler& scheduler, std::vector<std::shared_ptr<ICell<T>>> cells);
+            explicit WhenAllExecutionContext(std::vector<T> resolved_values) :
+                _resolved_values(resolved_values),
+                _num_resolved_cells(0),
+                _total_cells(resolved_values.size())
+            {}
 
-            auto read() const -> std::optional<std::vector<T>> override;
-            auto await(Callback<std::vector<T>> callback) -> void override;
-            auto block() -> std::vector<T> override;
+            [[nodiscard]] auto resolved_values() -> std::vector<T>& { return _resolved_values; }
+            [[nodiscard]] auto num_resolved_cells() -> std::atomic<size_t>& { return _num_resolved_cells; }
+            [[nodiscard]] auto total_cells() const -> size_t { return _total_cells; }
 
         private:
-            struct WhenAllExecutionContext {
-            public:
-                explicit WhenAllExecutionContext(std::vector<T> resolved_values) :
-                    _resolved_values(resolved_values),
-                    _num_resolved_cells(0),
-                    _total_cells(resolved_values.size())
-                {}
-
-                [[nodiscard]] auto resolved_values() -> std::vector<T>& { return _resolved_values; }
-                [[nodiscard]] auto num_resolved_cells() -> std::atomic<size_t>& { return _num_resolved_cells; }
-                [[nodiscard]] auto total_cells() const -> size_t { return _total_cells; }
-
-            private:
-                std::vector<T> _resolved_values;
-                std::atomic<size_t> _num_resolved_cells;
-                size_t _total_cells;
-            };
-
-
-            std::shared_ptr<WriteOnceCell<std::vector<T>>> underlying_cell;
-            std::vector<std::shared_ptr<ICell<T>>> cells;
+            std::vector<T> _resolved_values;
+            std::atomic<size_t> _num_resolved_cells;
+            size_t _total_cells;
+        };
+
+
+        std::shared_ptr<WriteOnceCell<std::vector<T>>> underlying_cell;
+        std::vector<std::shared_ptr<ICell<T>>> cells;
     };
 }
 
@@ -88,4 +88,4 @@ auto Cell::WhenAllCell<T>::await(Callback<std::vector<T>> callback) -> void {
 }
 
 template <typename T>
-auto Cell::WhenAllCell<T>::block() -> std::vector<T> { return underlying_cell->block(); }
+auto Cell::WhenAllCell<T>::block() const -> std::vector<T> { return underlying_cell->block(); }

src/cell/include/cell/when_any_cell.h

@@ -13,16 +13,16 @@ namespace Cell {
     // and resolves when any of the cells it is tracking resolves
     template <typename T>
     class WhenAnyCell : public ICell<T> {
-        public:
-            WhenAnyCell(Scheduler::IScheduler& scheduler, std::vector<std::shared_ptr<ICell<T>>> cells);
+    public:
+        WhenAnyCell(Scheduler::IScheduler& scheduler, std::vector<std::shared_ptr<ICell<T>>> cells);
 
-            auto read() const -> std::optional<T> override;
-            auto await(Callback<T> callback) -> void override;
-            auto block() -> T override;
+        [[nodiscard]] auto read() const -> std::optional<T> override;
+        [[nodiscard]] auto block() const -> T override;
+        auto await(Callback<T> callback) -> void override;
 
-        private:
-            std::shared_ptr<WriteOnceCell<T>> underlying_cell;
-            std::vector<std::shared_ptr<ICell<T>>> cells;
+    private:
+        std::shared_ptr<WriteOnceCell<T>> underlying_cell;
+        std::vector<std::shared_ptr<ICell<T>>> cells;
     };
 }
 
@@ -52,4 +52,4 @@ template <typename T>
 auto Cell::WhenAnyCell<T>::await(Callback<T> callback) -> void { underlying_cell->await(callback); }
 
 template <typename T>
-auto Cell::WhenAnyCell<T>::block() -> T { return underlying_cell->block(); }
+auto Cell::WhenAnyCell<T>::block() const -> T { return underlying_cell->block(); }

src/cell/include/cell/write_once_cell.h

@@ -14,40 +14,40 @@ namespace Cell {
     /// and read from multiple times it is thread-safe and can be awaited
     template <typename T>
     class WriteOnceCell : public ICell<T> {
-        public:
-            explicit WriteOnceCell(Scheduler::IScheduler& scheduler);
-
-            auto read() const -> std::optional<T> override;
-
-            // write performs a concurrent write to the WriteOnceCell
-            // it returns a boolean indicating if the write was successful
-            // a false indicates that the cell has already been written to
-            // and hence the write was not successful, there exists two overloaded methods
-            // for this fn, one method takes no scheduling context so uses an empty scheduling context
-            // when dispatching continuations and the other takes a defined context, for more information
-            // on scheduling contexts see the documentation under scheduler.h
-            auto write(T write_val) -> bool { return write(Scheduler::Context::empty(), write_val); }
-            auto write(Scheduler::Context ctx, T write_val) -> bool;
-
-            // await takes a callback function and calls it with the value
-            // of the WriteOnceCell when it is available.
-            auto await(Callback<T> callback) -> void override;
+    public:
+        explicit WriteOnceCell(Scheduler::IScheduler& scheduler);
+
+        [[nodiscard]] auto read() const -> std::optional<T> override;
+
+        // write performs a concurrent write to the WriteOnceCell
+        // it returns a boolean indicating if the write was successful
+        // a false indicates that the cell has already been written to
+        // and hence the write was not successful, there exists two overloaded methods
+        // for this fn, one method takes no scheduling context so uses an empty scheduling context
+        // when dispatching continuations and the other takes a defined context, for more information
+        // on scheduling contexts see the documentation under scheduler.h
+        auto write(T write_val) -> bool { return write(Scheduler::Context::empty(), write_val); }
+        auto write(Scheduler::Context ctx, T write_val) -> bool;
+
+        // await takes a callback function and calls it with the value
+        // of the WriteOnceCell when it is available.
+        auto await(Callback<T> callback) -> void override;
 
-            // block sleeps the current thread until the value of the cell is available
-            // it then returns the value of the cell, note that this is different from await
-            // as await registers a continuation, while block is a blocking operation
-            auto block() -> T override;
-
-        private:
-            mutable std::shared_mutex mutex;
-            mutable std::optional<T> value;
+        // block sleeps the current thread until the value of the cell is available
+        // it then returns the value of the cell, note that this is different from await
+        // as await registers a continuation, while block is a blocking operation
+        [[nodiscard]] auto block() const -> T override;
+
+    private:
+        mutable std::shared_mutex mutex;
+        mutable std::optional<T> value;
 
-            std::vector<Callback<T>> callbacks;
-            std::condition_variable_any cell_filled;
+        std::vector<Callback<T>> callbacks;
+        mutable std::condition_variable_any cell_filled;
 
-            //  Note: it is an invariant of the Asynchronous library that the scheduler's
-            //        is of 'static lifetime and hence will outlive any cell that uses it
-            std::reference_wrapper<Scheduler::IScheduler> scheduler;
+        //  Note: it is an invariant of the Asynchronous library that the scheduler's
+        //        is of 'static lifetime and hence will outlive any cell that uses it
+        std::reference_wrapper<Scheduler::IScheduler> scheduler;
     };
 }
 
@@ -109,7 +109,7 @@ auto Cell::WriteOnceCell<T>::await(Callback<T> callback) -> void {
 }
 
 template <typename T>
-auto Cell::WriteOnceCell<T>::block() -> T {
+auto Cell::WriteOnceCell<T>::block() const -> T {
     {
         std::shared_lock lock(mutex);
         if (!value.has_value()) {