refactor: code cleanup

asap-projects · Aug 20, 2022 · d2c79ff · d2c79ff
1 parent 05632a5
commit d2c79ff
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 131 deletions.
diff --git a/fsm/examples/tutorial/door_1.cpp b/fsm/examples/tutorial/door_1.cpp
@@ -4,10 +4,10 @@
 // SPDX-License-Identifier: BSD-3-Clause
 //===----------------------------------------------------------------------===//
 
-#include <fsm/fsm.h>
-
 #include <iostream>
 
+#include <fsm/fsm.h>
+
 using asap::fsm::ByDefault;
 using asap::fsm::DoNothing;
 using asap::fsm::On;

diff --git a/fsm/examples/tutorial/door_2.cpp b/fsm/examples/tutorial/door_2.cpp
@@ -4,10 +4,10 @@
 // SPDX-License-Identifier: BSD-3-Clause
 //===----------------------------------------------------------------------===//
 
-#include <fsm/fsm.h>
-
 #include <iostream>
 
+#include <fsm/fsm.h>
+
 using asap::fsm::ByDefault;
 using asap::fsm::Continue;
 using asap::fsm::DoNothing;
@@ -28,7 +28,8 @@ struct ClosedState
  : Will<ByDefault<DoNothing>, On<OpenEvent, TransitionTo<OpenState>>> {
  using Will::Handle;
 
- template <typename Event> auto OnEnter(const Event & /*event*/) -> Status {
+ template <typename Event>
+ static auto OnEnter(const Event & /*event*/) -> Status {
  std::cout << " > door is closed\n";
  return Continue{};
  }
@@ -43,12 +44,14 @@ struct OpenState
  : Will<ByDefault<DoNothing>, On<CloseEvent, TransitionTo<ClosedState>>> {
  using Will::Handle;
 
- template <typename Event> auto OnEnter(const Event & /*event*/) -> Status {
+ template <typename Event>
+ static auto OnEnter(const Event & /*event*/) -> Status {
  std::cout << " > door is open\n";
  return Continue{};
  }
 
- [[nodiscard]] static auto Handle(const OpenEvent & /*event*/) -> DoNothing {
+ [[nodiscard]] [[maybe_unused]] static auto Handle(const OpenEvent & /*event*/)
+ -> DoNothing {
  std::cerr << "Error: the door is already open!\n";
  return DoNothing{};
  }

diff --git a/fsm/examples/tutorial/door_3.cpp b/fsm/examples/tutorial/door_3.cpp
@@ -4,10 +4,10 @@
 // SPDX-License-Identifier: BSD-3-Clause
 //===----------------------------------------------------------------------===//
 
-#include <fsm/fsm.h>
-
 #include <iostream>
 
+#include <fsm/fsm.h>
+
 using asap::fsm::ByDefault;
 using asap::fsm::Continue;
 using asap::fsm::DoNothing;
@@ -39,17 +39,20 @@ struct ClosedState
  On<OpenEvent, TransitionTo<OpenState>>> {
  using Will::Handle;
 
- static auto OnEnter(const UnlockEvent & /*event*/) -> Status {
+ [[maybe_unused]] static auto OnEnter(const UnlockEvent & /*event*/)
+ -> Status {
  std::cout << " > door is closed - unlocked\n";
  return Continue{};
  }
 
- template <typename Event> auto OnEnter(const Event & /*event*/) -> Status {
+ template <typename Event>
+ static auto OnEnter(const Event & /*event*/) -> Status {
  std::cout << " > door is closed\n";
  return Continue{};
  }
 
- [[nodiscard]] static auto Handle(const CloseEvent & /*event*/) -> DoNothing {
+ [[nodiscard]] [[maybe_unused]] static auto Handle(
+ const CloseEvent & /*event*/) -> DoNothing {
  std::cerr << "Error: the door is already closed!\n";
  return DoNothing{};
  }
@@ -59,12 +62,14 @@ struct OpenState
  : Will<ByDefault<DoNothing>, On<CloseEvent, TransitionTo<ClosedState>>> {
  using Will::Handle;
 
- template <typename Event> auto OnEnter(const Event & /*event*/) -> Status {
+ template <typename Event>
+ static auto OnEnter(const Event & /*event*/) -> Status {
  std::cout << " > door is open\n";
  return Continue{};
  }
 
- [[nodiscard]] static auto Handle(const OpenEvent & /*event*/) -> DoNothing {
+ [[nodiscard]] [[maybe_unused]] static auto Handle(const OpenEvent & /*event*/)
+ -> DoNothing {
  std::cerr << "Error: the door is already open!\n";
  return DoNothing{};
  }
@@ -76,13 +81,13 @@ struct LockedState : ByDefault<DoNothing> {
  explicit LockedState(uint32_t key) : key_(key) {
  }
 
- auto OnEnter(const LockEvent &event) -> Status {
+ [[maybe_unused]] auto OnEnter(const LockEvent &event) -> Status {
  std::cout << " > door is locked with new code(" << event.newKey << ")\n";
  key_ = event.newKey;
  return Continue{};
  }
 
- [[nodiscard]] auto Handle(const UnlockEvent &event) const
+ [[nodiscard]] [[maybe_unused]] auto Handle(const UnlockEvent &event) const
  -> Maybe<TransitionTo<ClosedState>> {
  if (event.key == key_) {
  return TransitionTo<ClosedState>{};

diff --git a/fsm/include/fsm/fsm.h b/fsm/include/fsm/fsm.h
@@ -17,18 +17,16 @@
 
 #pragma once
 
-#include <fsm/asap_fsm_export.h>
-
 #include <any>
-#include <exception>
 #include <functional>
 #include <iostream>
-#include <optional>
 #include <tuple>
 #include <type_traits>
 #include <utility>
 #include <variant>
 
+#include <fsm/asap_fsm_export.h>
+
 /// Namespace for the State Machine library.
 namespace asap::fsm {
 
@@ -129,7 +127,7 @@ class ASAP_FSM_API StateMachineError {
  * \brief Called from derived exception classes to populate the error message
  * with meaningful information.
  */
- void What(std::string description);
+ void What(std::string description) const;
 
 private:
  // Internal implementation class.
@@ -364,28 +362,28 @@ template <typename TargetState> struct TransitionTo {
 private:
  // This overload is always in the set of overloads. Ellipsis parameter has the
  // lowest ranking for overload resolution.
- template <typename... Args> auto Leave(Args... /*unused*/) -> Status {
+ template <typename... Args> static auto Leave(Args... /*unused*/) -> Status {
  return Continue{};
  }
 
  // SFINAE: This overload is for states that implement `onLeave` with an event
  // parameter.
  template <typename State, typename Event>
- [[maybe_unused]] auto Leave(State &state, const Event &event)
+ [[maybe_unused]] static auto Leave(State &state, const Event &event)
  -> decltype(state.OnLeave(event)) {
  return state.OnLeave(event);
  }
 
  // This overload is always in the set of overloads. Ellipsis parameter has the
  // lowest ranking for overload resolution.
- template <typename... Args> auto Enter(Args... /*unused*/) -> Status {
+ template <typename... Args> static auto Enter(Args... /*unused*/) -> Status {
  return Continue{};
  }
 
  // SFINAE: This overload is for states that implement `onEnter` with an event
  // parameter. Data from the previous event, if any, will be discarded.
  template <typename State, typename Event>
- [[maybe_unused]] auto Enter(State &state, const Event &event)
+ [[maybe_unused]] static auto Enter(State &state, const Event &event)
  -> decltype(state.OnEnter(event)) {
  return state.OnEnter(event);
  }
@@ -417,7 +415,7 @@ template <typename TargetState> struct TransitionTo {
  */
 struct ASAP_FSM_API DoNothing {
  template <typename Machine, typename State, typename Event>
- auto Execute(Machine & /*unused*/, State & /*unused*/,
+ static auto Execute(Machine & /*unused*/, State & /*unused*/,
  const Event & /*unused*/) -> Status {
  return Continue{};
  }
@@ -517,8 +515,8 @@ constexpr auto supports_alternative() -> bool {
  */
 template <typename... Actions> struct OneOf {
  template <typename T,
- std::enable_if<!std::is_base_of<OneOf,
- typename std::remove_reference<T>::type>::value> * = nullptr>
+ std::enable_if<!std::is_base_of_v<OneOf, std::remove_reference_t<T>>> * =
+ nullptr>
  // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions,bugprone-forwarding-reference-overload)
  OneOf(T &&arg) : option(std::forward<T>(arg)) {
  }
@@ -616,7 +614,7 @@ struct is_one_of<Maybe<Actions...>> : std::true_type {};
  */
 template <typename Action> struct ByDefault {
  template <typename Event>
- auto Handle(const Event & /*unused*/) const -> Action {
+ static auto Handle(const Event & /*unused*/) -> Action {
  return Action{};
  }
 };
@@ -630,7 +628,7 @@ template <typename Action> struct ByDefault {
  * \snippet fsm_test.cpp On example
  */
 template <typename Event, typename Action> struct On {
- auto Handle(const Event & /*event*/) const -> Action {
+ static auto Handle(const Event & /*event*/) -> Action {
  return {};
  }
 };

diff --git a/fsm/src/fsm.cpp b/fsm/src/fsm.cpp
@@ -10,15 +10,17 @@
  * \brief Implementation details of the StateMachine and related types.
  */
 
-#include <common/compilers.h>
-#include <fsm/fsm.h>
+#include "fsm/fsm.h"
 
+#include <optional>
 #include <stdexcept>
 #include <utility>
 
+#include <common/compilers.h>
+
 // Warnings free memory allocation without full dependency on GSL implementation
 namespace gsl {
-template <class T, class = std::enable_if_t<std::is_pointer<T>::value>>
+template <class T, class = std::enable_if_t<std::is_pointer_v<T>>>
 using owner = T;
 } // namespace gsl
 
@@ -50,15 +52,16 @@ class StateMachineError::Impl {
 // warning :-)
 StateMachineError::Impl::~Impl() = default;
 
-StateMachineError::StateMachineError() : pimpl(gsl::owner<Impl *>(new Impl())) {
+StateMachineError::StateMachineError()
+ : pimpl(static_cast<gsl::owner<Impl *>>(new Impl())) {
 }
 
 StateMachineError::StateMachineError(std::string description)
  : pimpl(new Impl(std::move(description))) {
 }
 
 StateMachineError::StateMachineError(const StateMachineError &other)
- : pimpl(gsl::owner<Impl *>(new Impl(*other.pimpl))) {
+ : pimpl(static_cast<gsl::owner<Impl *>>(new Impl(*other.pimpl))) {
 }
 
 StateMachineError::StateMachineError(StateMachineError &&other) noexcept
@@ -72,7 +75,7 @@ auto StateMachineError::operator=(const StateMachineError &rhs)
  return *this;
  }
  delete pimpl;
- pimpl = gsl::owner<Impl *>(new Impl(*rhs.pimpl));
+ pimpl = static_cast<gsl::owner<Impl *>>(new Impl(*rhs.pimpl));
  return *this;
 }
 
@@ -99,7 +102,7 @@ auto StateMachineError::What() const -> const char * {
  return nullptr;
 }
 
-void StateMachineError::What(std::string description) {
+void StateMachineError::What(std::string description) const {
  pimpl->What(std::move(description));
 }
 

diff --git a/fsm/test/fsm_example_test.cpp b/fsm/test/fsm_example_test.cpp
@@ -4,28 +4,13 @@
 // SPDX-License-Identifier: BSD-3-Clause
 //===----------------------------------------------------------------------===//
 
-#include <fsm/fsm.h>
+#include "fsm/fsm.h"
 
-#include <common/compilers.h>
 #include <exception>
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
-// Disable compiler and linter warnings originating from the unit test framework
-// and for which we cannot do anything. Additionally, every TEST or TEST_X macro
-// usage must be preceded by a '// NOLINTNEXTLINE'.
-ASAP_DIAGNOSTIC_PUSH
-#if defined(__clang__) && ASAP_HAS_WARNING("-Wused-but-marked-unused")
-#pragma clang diagnostic ignored "-Wused-but-marked-unused"
-#pragma clang diagnostic ignored "-Wglobal-constructors"
-#pragma clang diagnostic ignored "-Wunused-member-function"
-#endif
-// NOLINTBEGIN(used-but-marked-unused)
-
-using testing::Eq;
-using testing::IsTrue;
-
 namespace asap::fsm {
 
 namespace {
@@ -65,13 +50,13 @@ struct LockedState : ByDefault<DoNothing> {
  explicit LockedState(uint32_t key) : key_(key) {
  }
 
- auto OnEnter(const LockEvent &event) -> Status {
+ [[maybe_unused]] auto OnEnter(const LockEvent &event) -> Status {
  key_ = event.newKey;
  return Continue{};
  }
 
  //! [State Handle method]
- [[nodiscard]] auto Handle(const UnlockEvent &event) const
+ [[nodiscard]] [[maybe_unused]] auto Handle(const UnlockEvent &event) const
  -> Maybe<TransitionTo<ClosedState>> {
  if (event.key == key_) {
  return TransitionTo<ClosedState>{};
@@ -102,6 +87,3 @@ TEST(StateMachine, ExampleTest) {
 } // namespace
 
 } // namespace asap::fsm
-
-// NOLINTEND(used-but-marked-unused)
-ASAP_DIAGNOSTIC_POP