Improve State class

- add additional error checks to State class
- extend tests for State class
- improve naming in State class tests
- adjust SerializableJsonStateMachine class

include/ls_std/logic/State.hpp

@@ -3,7 +3,7 @@
  * Company:         Lynar Studios
  * E-Mail:          webmaster@lynarstudios.com
  * Created:         2020-09-05
- * Changed:         2021-04-30
+ * Changed:         2021-05-28
  *
  * */
 
@@ -23,7 +23,7 @@ namespace ls_std
   {
     public:
 
-      explicit State(ls_std::StateId _id);
+      explicit State(const ls_std::StateId& _id);
       ~State() override = default;
 
       // implementation
@@ -39,8 +39,8 @@ namespace ls_std
       std::unordered_map<ls_std::StateConnectionId, std::shared_ptr<ls_std::StateConnection>> getConnectedStates();
       ls_std::StateId getId();
       bool hasConnection(const ls_std::StateConnectionId &_connectionId);
-      void setId(ls_std::StateId _id);
-      void setSerializable(std::shared_ptr<ls_std::ISerializable> _serializable);
+      void setId(const ls_std::StateId& _id);
+      void setSerializable(const std::shared_ptr<ls_std::ISerializable>& _serializable);
 
     private:
 
@@ -48,6 +48,8 @@ namespace ls_std
       ls_std::StateId id{};
       std::shared_ptr<ISerializable> serializable{};
 
+      void _assignSerializable(const std::shared_ptr<ls_std::ISerializable>& _serializable);
+      void _assignStateId(const ls_std::StateId& _id);
       void _clearConnections();
       bool _hasConnection(const ls_std::StateConnectionId &_connectionId);
   };

source/ls_std/logic/State.cpp

@@ -3,7 +3,7 @@
  * Company:         Lynar Studios
  * E-Mail:          webmaster@lynarstudios.com
  * Created:         2020-09-05
- * Changed:         2021-04-30
+ * Changed:         2021-05-28
  *
  * */
 
@@ -11,10 +11,10 @@
 #include <ls_std/exception/NullPointerException.hpp>
 #include <ls_std/exception/IllegalArgumentException.hpp>
 
-ls_std::State::State(ls_std::StateId _id)
-    : ls_std::Class("State"),
-      id(std::move(_id))
-{}
+ls_std::State::State(const ls_std::StateId& _id) : ls_std::Class("State")
+{
+  this->_assignStateId(_id);
+}
 
 ls_std::byte_field ls_std::State::marshal()
 {
@@ -49,11 +49,17 @@ bool ls_std::State::addStateConnection(const ls_std::StateConnectionId &_connect
   bool added{};
   std::shared_ptr<ls_std::StateConnection> connection{};
 
-  if (_connectedState != nullptr && !this->_hasConnection(_connectionId))
+  if (_connectionId.empty() || _connectedState == nullptr)
+  {
+    throw ls_std::IllegalArgumentException{};
+  }
+  else
   {
-    connection = std::make_shared<ls_std::StateConnection>(_connectionId, _connectedState->getId());
-    this->connectedStates.insert({_connectionId, connection});
-    added = true;
+    if (!this->_hasConnection(_connectionId))
+    {
+      connection = std::make_shared<ls_std::StateConnection>(_connectionId, _connectedState->getId());
+      added = this->connectedStates.insert({_connectionId, connection}).second;
+    }
   }
 
   return added;
@@ -65,8 +71,11 @@ bool ls_std::State::addStateConnection(const std::shared_ptr<ls_std::StateConnec
 
   if (_connection != nullptr)
   {
-    this->connectedStates.insert({_connection->getConnectionId(), _connection});
-    added = this->_hasConnection(_connection->getConnectionId());
+    added = this->connectedStates.insert({_connection->getConnectionId(), _connection}).second;
+  }
+  else
+  {
+    throw ls_std::IllegalArgumentException{};
   }
 
   return added;
@@ -92,19 +101,34 @@ bool ls_std::State::hasConnection(const ls_std::StateConnectionId &_connectionId
   return this->_hasConnection(_connectionId);
 }
 
-void ls_std::State::setId(ls_std::StateId _id)
+void ls_std::State::setId(const ls_std::StateId& _id)
 {
-  this->id = std::move(_id);
+  this->_assignStateId(_id);
 }
 
-void ls_std::State::setSerializable(std::shared_ptr<ls_std::ISerializable> _serializable)
+void ls_std::State::setSerializable(const std::shared_ptr<ls_std::ISerializable>& _serializable)
+{
+  this->_assignSerializable(_serializable);
+}
+
+void ls_std::State::_assignSerializable(const std::shared_ptr<ls_std::ISerializable> &_serializable)
 {
   if (_serializable == nullptr)
   {
     throw ls_std::IllegalArgumentException{};
   }
 
-  this->serializable = std::move(_serializable);
+  this->serializable = _serializable;
+}
+
+void ls_std::State::_assignStateId(const ls_std::StateId &_id)
+{
+  if (_id.empty())
+  {
+    throw ls_std::IllegalArgumentException{};
+  }
+
+  this->id = _id;
 }
 
 void ls_std::State::_clearConnections()

source/ls_std/serialization/json/logic/SerializableJsonStateMachine.cpp

@@ -3,7 +3,7 @@
  * Company:         Lynar Studios
  * E-Mail:          webmaster@lynarstudios.com
  * Created:         2020-09-17
- * Changed:         2021-05-02
+ * Changed:         2021-05-28
  *
  * */
 
@@ -64,7 +64,7 @@ void ls_std::SerializableJsonStateMachine::_unmarshalStates()
 {
   for (const auto &serializedState : this->jsonObject["states"])
   {
-    std::shared_ptr<ls_std::State> state = std::make_shared<ls_std::State>("");
+    std::shared_ptr<ls_std::State> state = std::make_shared<ls_std::State>("TMP_ID");
     ls_std::SerializableJsonState{state}.unmarshal(serializedState.dump());
     this->value->addState(state);
   }

test/cases/logic/StateTest.cpp

@@ -3,7 +3,7 @@
  * Company:         Lynar Studios
  * E-Mail:          webmaster@lynarstudios.com
  * Created:         2020-09-05
- * Changed:         2021-05-02
+ * Changed:         2021-05-28
  *
  * */
 
@@ -27,6 +27,26 @@ namespace
       {}
   };
 
+  TEST_F(StateTest, getClassName)
+  {
+    ls_std::State state{"A"};
+    ASSERT_STREQ("State", state.getClassName().c_str());
+  }
+
+  TEST_F(StateTest, constructor_empty_id)
+  {
+    EXPECT_THROW({
+                   try
+                   {
+                     ls_std::State state = ls_std::State("");
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
   // implementation
 
   TEST_F(StateTest, marshal)
@@ -38,7 +58,7 @@ namespace
     ASSERT_FALSE(state->marshal().empty());
   }
 
-  TEST_F(StateTest, marshal_noSerializableReference)
+  TEST_F(StateTest, marshal_no_serializable_reference)
   {
     ls_std::State state{"A"};
 
@@ -65,7 +85,7 @@ namespace
     ASSERT_STREQ("A", state->getId().c_str());
   }
 
-  TEST_F(StateTest, unmarshal_noSerializableReference)
+  TEST_F(StateTest, unmarshal_no_serializable_reference)
   {
     std::shared_ptr<ls_std::State> state = std::make_shared<ls_std::State>("TMP_ID");
     std::string jsonString = R"({"id":"A","connectedStates":{"AB":{"condition":false,"connectionId":"AB","stateId":"B"}}})";
@@ -84,7 +104,7 @@ namespace
 
   // additional functionality
 
-  TEST_F(StateTest, addStateConnection)
+  TEST_F(StateTest, addStateConnection_v1)
   {
     ls_std::State stateA{"A"};
     ls_std::State stateB{"B"};
@@ -92,17 +112,48 @@ namespace
     ASSERT_TRUE(stateA.addStateConnection("AB", std::make_shared<ls_std::State>(stateB)));
   }
 
-  TEST_F(StateTest, addStateConnectionNegative)
+  TEST_F(StateTest, addStateConnection_v1_connection_already_exists)
   {
     ls_std::State stateA{"A"};
     ls_std::State stateB{"B"};
 
     ASSERT_TRUE(stateA.addStateConnection("AB", std::make_shared<ls_std::State>(stateB)));
     ASSERT_FALSE(stateA.addStateConnection("AB", std::make_shared<ls_std::State>(stateB)));
-    ASSERT_FALSE(stateA.addStateConnection("XX", nullptr));
   }
 
-  TEST_F(StateTest, addStateConnectionV2)
+  TEST_F(StateTest, addStateConnection_v1_empty_connection_id)
+  {
+    ls_std::State state{"A"};
+
+    EXPECT_THROW({
+                   try
+                   {
+                     state.addStateConnection("", std::make_shared<ls_std::State>("B"));
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
+  TEST_F(StateTest, addStateConnection_v1_no_reference)
+  {
+    ls_std::State state{"A"};
+
+    EXPECT_THROW({
+                   try
+                   {
+                     state.addStateConnection("AB", nullptr);
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
+  TEST_F(StateTest, addStateConnection_v2)
   {
     ls_std::State stateA{"A"};
     ls_std::State stateB{"B"};
@@ -110,10 +161,20 @@ namespace
     ASSERT_TRUE(stateA.addStateConnection(std::make_shared<ls_std::StateConnection>("AB", stateB.getId())));
   }
 
-  TEST_F(StateTest, addStateConnectionV2Negative)
+  TEST_F(StateTest, addStateConnection_v2_no_reference)
   {
     ls_std::State stateA{"A"};
-    ASSERT_FALSE(stateA.addStateConnection(nullptr));
+
+    EXPECT_THROW({
+                   try
+                   {
+                     stateA.addStateConnection(nullptr);
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
   }
 
   TEST_F(StateTest, clearConnections)
@@ -122,28 +183,15 @@ namespace
     stateA.addStateConnection(std::make_shared<ls_std::StateConnection>("AB", "B"));
     stateA.addStateConnection(std::make_shared<ls_std::StateConnection>("AC", "C"));
 
-    ASSERT_EQ(2, stateA.getConnectedStates().size());
+    ASSERT_FALSE(stateA.getConnectedStates().empty());
     stateA.clearConnections();
-    ASSERT_EQ(0, stateA.getConnectedStates().size());
     ASSERT_TRUE(stateA.getConnectedStates().empty());
   }
 
   TEST_F(StateTest, getConnectedStates)
   {
     ls_std::State stateA{"A"};
-    ls_std::State stateB{"B"};
-
-    ASSERT_TRUE(stateA.addStateConnection("AB", std::make_shared<ls_std::State>(stateB)));
-    ASSERT_FALSE(stateA.getConnectedStates().empty());
-    ASSERT_EQ(1, stateA.getConnectedStates().size());
-  }
-
-  TEST_F(StateTest, getConnectedStatesNegative)
-  {
-    ls_std::State stateA{"A"};
-
     ASSERT_TRUE(stateA.getConnectedStates().empty());
-    ASSERT_EQ(0, stateA.getConnectedStates().size());
   }
 
   TEST_F(StateTest, getId)
@@ -161,6 +209,22 @@ namespace
     ASSERT_STREQ("B", stateA.getId().c_str());
   }
 
+  TEST_F(StateTest, setId_empty_id)
+  {
+    ls_std::State stateA{"A"};
+
+    EXPECT_THROW({
+                   try
+                   {
+                     stateA.setId("");
+                   }
+                   catch (const ls_std::IllegalArgumentException &_exception)
+                   {
+                     throw;
+                   }
+                 }, ls_std::IllegalArgumentException);
+  }
+
   TEST_F(StateTest, hasConnection)
   {
     ls_std::State stateA{"A"};
@@ -173,22 +237,20 @@ namespace
     ASSERT_TRUE(stateA.hasConnection("AC"));
   }
 
-  TEST_F(StateTest, hasConnectionNegative)
+  TEST_F(StateTest, hasConnection_no_connections_available)
   {
     ls_std::State stateA{"A"};
     ASSERT_FALSE(stateA.hasConnection("AB"));
-    ASSERT_FALSE(stateA.hasConnection("AC"));
   }
 
-  TEST_F(StateTest, setSerializable_noSerializableReference)
+  TEST_F(StateTest, setSerializable_no_reference)
   {
     ls_std::State state{"A"};
-    std::shared_ptr<ls_std::ISerializable> serializable{};
 
     EXPECT_THROW({
                    try
                    {
-                     state.setSerializable(serializable);
+                     state.setSerializable(nullptr);
                    }
                    catch (const ls_std::IllegalArgumentException &_exception)
                    {

test/cases/serialization/json/logic/SerializableJsonStateMachineTest.cpp

@@ -3,7 +3,7 @@
  * Company:         Lynar Studios
  * E-Mail:          webmaster@lynarstudios.com
  * Created:         2020-09-17
- * Changed:         2021-05-02
+ * Changed:         2021-05-28
  *
  * */
 
@@ -51,54 +51,54 @@ namespace
   {
     ls_std::File file{TestHelper::getResourcesFolderLocation() + "/state_machine_test.json"};
     ls_std::FileReader reader{file};
-    std::shared_ptr<ls_std::StateMachine> x = std::make_shared<ls_std::StateMachine>("bla");
-    ls_std::SerializableJsonStateMachine serializable{x};
+    std::shared_ptr<ls_std::StateMachine> machine = std::make_shared<ls_std::StateMachine>("bla");
+    ls_std::SerializableJsonStateMachine serializable{machine};
 
     serializable.unmarshal(reader.read());
 
-    ASSERT_STREQ("test_machine", x->getName().c_str());
+    ASSERT_STREQ("test_machine", machine->getName().c_str());
 
     // check memory
 
-    ASSERT_FALSE(x->getMemory().empty());
-    ASSERT_EQ(3, x->getMemory().size());
-    ASSERT_STREQ("A", x->getMemory().at(0).c_str());
-    ASSERT_STREQ("B", x->getMemory().at(1).c_str());
-    ASSERT_STREQ("C", x->getMemory().at(2).c_str());
+    ASSERT_FALSE(machine->getMemory().empty());
+    ASSERT_EQ(3, machine->getMemory().size());
+    ASSERT_STREQ("A", machine->getMemory().at(0).c_str());
+    ASSERT_STREQ("B", machine->getMemory().at(1).c_str());
+    ASSERT_STREQ("C", machine->getMemory().at(2).c_str());
 
     // check current state
 
-    ASSERT_TRUE(x->getCurrentState() != nullptr);
-    ASSERT_STREQ("A", x->getCurrentState()->getId().c_str());
+    ASSERT_TRUE(machine->getCurrentState() != nullptr);
+    ASSERT_STREQ("A", machine->getCurrentState()->getId().c_str());
 
     // check states
 
-    ASSERT_TRUE(!x->getStates().empty());
-    ASSERT_EQ(5, x->getStates().size());
+    ASSERT_TRUE(!machine->getStates().empty());
+    ASSERT_EQ(5, machine->getStates().size());
 
-    std::shared_ptr<ls_std::State> state = x->getStates().at("A");
+    std::shared_ptr<ls_std::State> state = machine->getStates().at("A");
     ASSERT_STREQ("A", state->getId().c_str());
     ASSERT_EQ(1, state->getConnectedStates().size());
     ASSERT_STREQ("AB", state->getConnectedStates().at("AB")->getConnectionId().c_str());
 
-    state = x->getStates().at("B");
+    state = machine->getStates().at("B");
     ASSERT_STREQ("B", state->getId().c_str());
     ASSERT_EQ(2, state->getConnectedStates().size());
     ASSERT_STREQ("BC", state->getConnectedStates().at("BC")->getConnectionId().c_str());
     ASSERT_STREQ("BD", state->getConnectedStates().at("BD")->getConnectionId().c_str());
 
-    state = x->getStates().at("C");
+    state = machine->getStates().at("C");
     ASSERT_STREQ("C", state->getId().c_str());
     ASSERT_EQ(2, state->getConnectedStates().size());
     ASSERT_STREQ("CB", state->getConnectedStates().at("CB")->getConnectionId().c_str());
     ASSERT_STREQ("CE", state->getConnectedStates().at("CE")->getConnectionId().c_str());
 
-    state = x->getStates().at("D");
+    state = machine->getStates().at("D");
     ASSERT_STREQ("D", state->getId().c_str());
     ASSERT_EQ(1, state->getConnectedStates().size());
     ASSERT_STREQ("DE", state->getConnectedStates().at("DE")->getConnectionId().c_str());
 
-    state = x->getStates().at("E");
+    state = machine->getStates().at("E");
     ASSERT_STREQ("E", state->getId().c_str());
     ASSERT_TRUE(state->getConnectedStates().empty());
   }