Improve StateMachine class

- add additional error checks to StateMachine class
- extend tests for StateMachine class
- improve naming in StateMachine class tests

include/ls_std/logic/StateMachine.hpp

@@ -24,7 +24,7 @@ namespace ls_std
   {
     public:
 
-      explicit StateMachine(std::string _name);
+      explicit StateMachine(const std::string& _name);
       ~StateMachine() override = default;
 
       bool addState(const std::shared_ptr<ls_std::State> &_state);
@@ -34,8 +34,8 @@ namespace ls_std
       std::unordered_map<StateId, std::shared_ptr<ls_std::State>> getStates();
       bool hasState(const ls_std::StateId &_id);
       bool proceed();
-      void setMemory(std::vector<ls_std::StateId> _memory);
-      void setName(std::string _name);
+      void setMemory(const std::vector<ls_std::StateId>& _memory);
+      void setName(const std::string& _name);
       bool setStartState(const ls_std::StateId &_id);
 
     private:
@@ -45,6 +45,8 @@ namespace ls_std
       std::string name{};
       std::unordered_map<ls_std::StateId, std::shared_ptr<ls_std::State>> states{};
 
+      void _assignMemory(const std::vector<ls_std::StateId>& _memory);
+      void _assignName(const std::string& _name);
       std::vector<ls_std::StateId> _getNextValidStates();
       void _remember(const ls_std::StateId &_id);
       bool _hasState(const ls_std::StateId &_id);

source/ls_std/logic/StateMachine.cpp

@@ -8,23 +8,30 @@
  * */
 
 #include <ls_std/logic/StateMachine.hpp>
+#include <ls_std/exception/IllegalArgumentException.hpp>
 
-ls_std::StateMachine::StateMachine(std::string _name)
-    : ls_std::Class("StateMachine"),
-      name(std::move(_name))
-{}
+ls_std::StateMachine::StateMachine(const std::string& _name) : ls_std::Class("StateMachine")
+{
+  this->_assignName(_name);
+}
 
 bool ls_std::StateMachine::addState(const std::shared_ptr<ls_std::State> &_state)
 {
-  bool condition = !this->_hasState(_state->getId());
+  bool wasAdded{};
 
-  if (condition)
+  if (_state == nullptr)
+  {
+    throw ls_std::IllegalArgumentException{};
+  }
+  else
   {
-    this->states.insert({_state->getId(), _state});
-    condition = this->_hasState(_state->getId());
+    if (!this->_hasState(_state->getId()))
+    {
+      wasAdded = this->states.insert({_state->getId(), _state}).second;
+    }
   }
 
-  return condition;
+  return wasAdded;
 }
 
 std::shared_ptr<ls_std::State> ls_std::StateMachine::getCurrentState()
@@ -55,38 +62,66 @@ bool ls_std::StateMachine::hasState(const ls_std::StateId &_id)
 bool ls_std::StateMachine::proceed()
 {
   std::vector<ls_std::StateId> nextValidStates = this->_getNextValidStates();
-  bool condition = nextValidStates.size() == 1;
+  bool onlyOneWayToGo = nextValidStates.size() == 1;
 
-  if (condition)
+  if (onlyOneWayToGo)
   {
     this->currentState = this->states[nextValidStates.at(0)];
     this->_remember(nextValidStates.at(0));
   }
 
-  return condition;
+  return onlyOneWayToGo;
 }
 
-void ls_std::StateMachine::setMemory(std::vector<ls_std::StateId> _memory)
+void ls_std::StateMachine::setMemory(const std::vector<ls_std::StateId>& _memory)
 {
-  this->memory = std::move(_memory);
+  this->_assignMemory(_memory);
 }
 
-void ls_std::StateMachine::setName(std::string _name)
+void ls_std::StateMachine::setName(const std::string& _name)
 {
-  this->name = std::move(_name);
+  this->_assignName(_name);
 }
 
 bool ls_std::StateMachine::setStartState(const ls_std::StateId &_id)
 {
-  bool exists = this->_hasState(_id);
+  bool startStateSet{};
+
+  if (_id.empty())
+  {
+    throw ls_std::IllegalArgumentException{};
+  }
+  else
+  {
+    if (this->_hasState(_id))
+    {
+      this->currentState = this->states[_id];
+      this->_remember(_id);
+      startStateSet = true;
+    }
+  }
+
+  return startStateSet;
+}
+
+void ls_std::StateMachine::_assignMemory(const std::vector<ls_std::StateId> &_memory)
+{
+  if (_memory.empty())
+  {
+    throw ls_std::IllegalArgumentException{};
+  }
 
-  if (exists)
+  this->memory = _memory;
+}
+
+void ls_std::StateMachine::_assignName(const std::string &_name)
+{
+  if (_name.empty())
   {
-    this->currentState = this->states[_id];
-    this->_remember(_id);
+    throw ls_std::IllegalArgumentException{};
   }
 
-  return exists;
+  this->name = _name;
 }
 
 std::vector<ls_std::StateId> ls_std::StateMachine::_getNextValidStates()

test/cases/logic/StateMachineTest.cpp

@@ -3,7 +3,7 @@
  * Company:         Lynar Studios
  * E-Mail:          webmaster@lynarstudios.com
  * Created:         2020-09-09
- * Changed:         2021-04-23
+ * Changed:         2021-05-28
  *
  * */
 
@@ -27,19 +27,54 @@ namespace
       {}
   };
 
-  TEST_F(StateMachineTest, addStateConnection)
+  TEST_F(StateMachineTest, getClassName)
+  {
+    ls_std::StateMachine stateMachine {"machine"};
+    ASSERT_STREQ("StateMachine", stateMachine.getClassName().c_str());
+  }
+
+  TEST_F(StateMachineTest, constructor_empty_name)
+  {
+    EXPECT_THROW({
+                   try
+                   {
+                     ls_std::StateMachine stateMachine {""};
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
+  TEST_F(StateMachineTest, addState)
   {
     ls_std::StateMachine stateMachine{"test_machine"};
     ASSERT_TRUE(stateMachine.addState(std::make_shared<ls_std::State>("A")));
   }
 
-  TEST_F(StateMachineTest, addStateConnectionNegative)
+  TEST_F(StateMachineTest, addState_state_already_exists)
   {
     ls_std::StateMachine stateMachine{"test_machine"};
     ASSERT_TRUE(stateMachine.addState(std::make_shared<ls_std::State>("A")));
     ASSERT_FALSE(stateMachine.addState(std::make_shared<ls_std::State>("A")));
   }
 
+  TEST_F(StateMachineTest, addState_no_reference)
+  {
+    EXPECT_THROW({
+                   try
+                   {
+                     ls_std::StateMachine stateMachine{"test_machine"};
+                     stateMachine.addState(nullptr);
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
   TEST_F(StateMachineTest, getCurrentState)
   {
     ls_std::StateMachine stateMachine{"test_machine"};
@@ -50,38 +85,8 @@ namespace
 
   TEST_F(StateMachineTest, getMemory)
   {
-    ls_std::StateMachine stateMachine = ls_std_test::TestDataFactory::createStateMachine();
-    ASSERT_STREQ("test_machine", stateMachine.getName().c_str());
-    ASSERT_EQ(0, stateMachine.getMemory().size());
-
-    stateMachine.setStartState("A");
-    ASSERT_EQ(1, stateMachine.getMemory().size());
-    ASSERT_STREQ("A", stateMachine.getMemory().at(0).c_str());
-
-    ASSERT_FALSE(stateMachine.proceed());
-    ASSERT_EQ(1, stateMachine.getMemory().size());
-    ASSERT_STREQ("A", stateMachine.getMemory().at(0).c_str());
-
-    stateMachine.getCurrentState()->getConnectedStates().at("AB")->updatePassCondition(true);
-    ASSERT_TRUE(stateMachine.proceed());
-    ASSERT_EQ(2, stateMachine.getMemory().size());
-    ASSERT_STREQ("A", stateMachine.getMemory().at(0).c_str());
-    ASSERT_STREQ("B", stateMachine.getMemory().at(1).c_str());
-
-    stateMachine.getCurrentState()->getConnectedStates().at("BC")->updatePassCondition(true);
-    ASSERT_TRUE(stateMachine.proceed());
-    ASSERT_EQ(3, stateMachine.getMemory().size());
-    ASSERT_STREQ("A", stateMachine.getMemory().at(0).c_str());
-    ASSERT_STREQ("B", stateMachine.getMemory().at(1).c_str());
-    ASSERT_STREQ("C", stateMachine.getMemory().at(2).c_str());
-
-    stateMachine.getCurrentState()->getConnectedStates().at("CB")->updatePassCondition(true);
-    ASSERT_TRUE(stateMachine.proceed());
-    ASSERT_EQ(4, stateMachine.getMemory().size());
-    ASSERT_STREQ("A", stateMachine.getMemory().at(0).c_str());
-    ASSERT_STREQ("B", stateMachine.getMemory().at(1).c_str());
-    ASSERT_STREQ("C", stateMachine.getMemory().at(2).c_str());
-    ASSERT_STREQ("B", stateMachine.getMemory().at(3).c_str());
+    ls_std::StateMachine stateMachine {"machine"};
+    ASSERT_TRUE(stateMachine.getMemory().empty());
   }
 
   TEST_F(StateMachineTest, getName)
@@ -92,10 +97,8 @@ namespace
 
   TEST_F(StateMachineTest, getStates)
   {
-    ls_std::StateMachine stateMachine = ls_std_test::TestDataFactory::createStateMachine();
-
-    ASSERT_TRUE(!stateMachine.getStates().empty());
-    ASSERT_EQ(5, stateMachine.getStates().size());
+    ls_std::StateMachine stateMachine {"machine"};
+    ASSERT_TRUE(stateMachine.getStates().empty());
   }
 
   TEST_F(StateMachineTest, hasState)
@@ -109,9 +112,9 @@ namespace
     ASSERT_TRUE(stateMachine.hasState("E"));
   }
 
-  TEST_F(StateMachineTest, hasStateNegative)
+  TEST_F(StateMachineTest, hasState_no_state_available)
   {
-    ls_std::StateMachine stateMachine = ls_std_test::TestDataFactory::createStateMachine();
+    ls_std::StateMachine stateMachine {"machine"};
     ASSERT_FALSE(stateMachine.hasState("F"));
   }
 
@@ -166,19 +169,21 @@ namespace
     ASSERT_STREQ("E", stateMachine.getCurrentState()->getId().c_str());
   }
 
-  TEST_F(StateMachineTest, setMemory)
+  TEST_F(StateMachineTest, setMemory_no_memory)
   {
     ls_std::StateMachine stateMachine{"test_machine"};
-    ASSERT_TRUE(stateMachine.getMemory().empty());
-
-    std::vector<ls_std::StateId> memory{"A", "B", "C"};
-    stateMachine.setMemory(memory);
-
-    ASSERT_FALSE(stateMachine.getMemory().empty());
-    ASSERT_EQ(3, stateMachine.getMemory().size());
-    ASSERT_STREQ("A", stateMachine.getMemory().at(0).c_str());
-    ASSERT_STREQ("B", stateMachine.getMemory().at(1).c_str());
-    ASSERT_STREQ("C", stateMachine.getMemory().at(2).c_str());
+    std::vector<ls_std::StateId> memory{};
+
+    EXPECT_THROW({
+                   try
+                   {
+                     stateMachine.setMemory(memory);
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
   }
 
   TEST_F(StateMachineTest, setName)
@@ -190,13 +195,50 @@ namespace
     ASSERT_STREQ("bla", stateMachine.getName().c_str());
   }
 
+  TEST_F(StateMachineTest, setName_empty_name)
+  {
+    EXPECT_THROW({
+                   try
+                   {
+                     ls_std::StateMachine stateMachine {"machine"};
+                     stateMachine.setName("");
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
   TEST_F(StateMachineTest, setStartState)
   {
     ls_std::StateMachine stateMachine{"test_machine"};
-    ASSERT_FALSE(stateMachine.getCurrentState() != nullptr);
+    ASSERT_TRUE(stateMachine.getCurrentState() == nullptr);
+    stateMachine.addState(std::make_shared<ls_std::State>("A"));
 
-    ASSERT_TRUE(stateMachine.addState(std::make_shared<ls_std::State>("A")));
     ASSERT_TRUE(stateMachine.setStartState("A"));
-    ASSERT_TRUE(stateMachine.getCurrentState() != nullptr);
+    ASSERT_FALSE(stateMachine.getCurrentState() == nullptr);
+  }
+
+  TEST_F(StateMachineTest, setStartState_state_does_not_exist)
+  {
+    ls_std::StateMachine stateMachine{"test_machine"};
+    ASSERT_FALSE(stateMachine.setStartState("A"));
+  }
+
+  TEST_F(StateMachineTest, setStartState_empty_id)
+  {
+    ls_std::StateMachine stateMachine{"test_machine"};
+
+    EXPECT_THROW({
+                   try
+                   {
+                     stateMachine.setStartState("");
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
   }
 }