From 807648b65ac27404e607d7299b3d9d948af33093 Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Fri, 23 Aug 2019 23:56:46 +0300
Subject: [PATCH] Replace exception usage with if statements
---
inc/ErrorHandler.hpp | 8 +++--
inc/Services/Parameter.hpp | 2 +-
inc/Services/ParameterService.hpp | 7 ++--
src/Services/FunctionManagementService.cpp | 2 +-
src/Services/Parameter.cpp | 2 +-
src/Services/ParameterService.cpp | 38 ++++++++++-----------
test/Services/FunctionManagementService.cpp | 2 +-
test/Services/ParameterService.cpp | 11 +++---
8 files changed, 38 insertions(+), 34 deletions(-)
diff --git a/inc/ErrorHandler.hpp b/inc/ErrorHandler.hpp
index 97732f38..5dd1c6fb 100644
--- a/inc/ErrorHandler.hpp
+++ b/inc/ErrorHandler.hpp
@@ -70,9 +70,13 @@ public:
*/
OtherMessageType = 9,
/**
- * Attempt to insert new function in a full function map (ST[08])
+ * Attempt to insert new element in a full map (ST[08], ST[20])
*/
- FunctionMapFull = 10,
+ MapFull = 10,
+ /**
+ * Attempt to overwrite an existing parameter (ST[20])
+ */
+ ExistingParameterId = 11
};
/**
diff --git a/inc/Services/Parameter.hpp b/inc/Services/Parameter.hpp
index 2cec0f88..54a40bb1 100644
--- a/inc/Services/Parameter.hpp
+++ b/inc/Services/Parameter.hpp
@@ -66,7 +66,7 @@ class Parameter {
Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue = 0, UpdatePtr newPtr = nullptr);
void setCurrentValue(ValueType newVal);
- void setFlag(const char* flags);
+ void setFlags(const char* flags);
ValueType getCurrentValue();
uint8_t getPTC();
diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index fddef4f6..3d473103 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -36,13 +36,14 @@ public:
ParameterService();
/**
- * @brief Adds a new parameter. Returns false if the parameter has not been added
- * (either because the map is full or because it already exists in it).
+ * @brief Adds a new parameter. Emits an InternalError::MapFull if an attempt is made to insert
+ * parameters in a full map or an InternalError::ExistingParameterId if the given parameter ID
+ * exists already.
* @param id: the desired ID for this parameter
* @param param: the parameter field to be included
* @param flags: the flags to be set for this field (see Parameter.hpp)
*/
- bool addNewParameter(uint16_t id, Parameter param, const char* flags = "110");
+ void addNewParameter(uint16_t id, Parameter param, const char* flags = "110");
/**
* This function receives a TC[20, 1] packet and returns a TM[20, 2] packet
diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp
index c3831872..b6d5638b 100644
--- a/src/Services/FunctionManagementService.cpp
+++ b/src/Services/FunctionManagementService.cpp
@@ -42,7 +42,7 @@ void FunctionManagementService::include(String<FUNC_NAME_LENGTH> funcName, void
funcName.append(FUNC_NAME_LENGTH - funcName.length(), 0);
funcPtrIndex.insert(std::make_pair(funcName, ptr));
} else {
- ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::FunctionMapFull);
+ ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::MapFull);
}
}
diff --git a/src/Services/Parameter.cpp b/src/Services/Parameter.cpp
index fae89171..2ab92271 100644
--- a/src/Services/Parameter.cpp
+++ b/src/Services/Parameter.cpp
@@ -34,6 +34,6 @@ uint8_t Parameter::getPFC() {
return pfc;
}
-void Parameter::setFlag(const char* flags) {
+void Parameter::setFlags(const char* flags) {
this->flags = Flags(flags);
}
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index 9058e046..2ac5d101 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -7,24 +7,21 @@ ParameterService::ParameterService() {
// addNewParameter(3, 14);
}
-bool ParameterService::addNewParameter(uint16_t id, Parameter param, const char* flags) {
- try {
- try {
- paramsList.at(id);
- return false;
- // if it exists, an iterator will be returned with no exception thrown,
- // so the function should return false
- // todo: is it a better idea to use find() and if instead of an exception here?
- }
- catch (etl::map_out_of_bounds& mapOutOfBounds) {
- param.setFlag(flags);
+void ParameterService::addNewParameter(uint16_t id, Parameter param, const char* flags) {
+ if (paramsList.full()) {
+ ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::MapFull);
+ return;
+ }
+ else {
+ if (paramsList.find(id) == paramsList.end()) {
+ param.setFlags(flags);
paramsList.insert(std::make_pair(id, param));
- return true;
+ return;
+ }
+ else {
+ ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::ExistingParameterId);
+ return;
}
- }
- catch (etl::map_full &mapFull) {
- return false;
- // if the map is full, return false
}
}
@@ -51,13 +48,14 @@ void ParameterService::reportParameterIds(Message& paramIds) {
for (uint16_t i = 0; i < numOfIds; i++) {
uint16_t currId = paramIds.readUint16();
- try {
+
+ if (paramsList.find(currId) != paramsList.end()) {
std::pair<uint16_t, ValueType> p = std::make_pair(currId, paramsList.at(currId).getCurrentValue());
// pair containing the parameter's ID as first element and its current value as second
validParams.push_back(p);
validIds++;
}
- catch (etl::map_out_of_bounds &mapOutOfBounds) {
+ else {
ErrorHandler::reportError(paramIds, ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError);
continue; // generate failed start of execution notification & ignore
}
@@ -89,10 +87,10 @@ void ParameterService::setParameterIds(Message& newParamValues) {
for (uint16_t i = 0; i < numOfIds; i++) {
uint16_t currId = newParamValues.readUint16();
// the parameter is checked for read-only status and manual update availability
- try {
+ if (paramsList.find(currId) != paramsList.end()) {
paramsList.at(currId).setCurrentValue(newParamValues.readUint32());
}
- catch (etl::map_out_of_bounds &mapOutOfBounds) {
+ else {
ErrorHandler::reportError(newParamValues,
ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError);
continue; // generate failed start of execution notification & ignore
diff --git a/test/Services/FunctionManagementService.cpp b/test/Services/FunctionManagementService.cpp
index 45088a86..64683103 100644
--- a/test/Services/FunctionManagementService.cpp
+++ b/test/Services/FunctionManagementService.cpp
@@ -71,6 +71,6 @@ TEST_CASE("ST[08] - Insert Tests") {
name += std::to_string(i); // different names to fill up the map
fms.include(String<FUNC_NAME_LENGTH>(name.c_str()), &test);
}
- CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::FunctionMapFull));
+ CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::MapFull));
}
}
diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp
index c7b3c267..ee678cd5 100644
--- a/test/Services/ParameterService.cpp
+++ b/test/Services/ParameterService.cpp
@@ -26,7 +26,9 @@ TEST_CASE("Parameter Service - General") {
pserv.addNewParameter(3, param3);
pserv.addNewParameter(4, param4);
- REQUIRE_FALSE(pserv.addNewParameter(5, param5)); // addNewParameter should return false
+ pserv.addNewParameter(5, param5); // addNewParameter should return false
+ CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::MapFull));
+ ServiceTests::reset();
Services.reset(); // reset all services
}
@@ -34,12 +36,11 @@ TEST_CASE("Parameter Service - General") {
Parameter param0 = Parameter(1, 3);
pserv.addNewParameter(0, param0);
- CHECK_FALSE(pserv.addNewParameter(0, param0));
+ pserv.addNewParameter(0, param0);
+ CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::ExistingParameterId));
+ ServiceTests::reset();
Services.reset();
}
-
- //SECTION("Passing of null-pointer as update function on construction")
-
}
TEST_CASE("Parameter Report Subservice") {
--
GitLab