This is the first time I am using C++ templates in a very long time, and is the first time I have created a template class.
When developing projects I often run into a common problem: I have an enum that I need strings for, either for display purposes or for testing and debugging. In the project I'm currently developing, I encountered this for 2 different enums. Initially I created 2 different classes to do the translation using 2 different implementations. I am now trying to refactor that code to reduce the amount of code in the project (currently 1530 lines of code). I initially used std::map in one of the implementations, but, in a previous review I was informed that there is a cost to using std::map.
I would ask if there were memory leaks, but Valgrind assures me there are no memory leaks. Running Valgrind did help find 2 errors in the code that have been corrected.
The code provided is a standalone unit test.
Questions I'd appreciate answers to:
- Using the C++ 20 standard, are there any additional features that I can use to reduce the amount of code needed in either the
DictionaryBaseclass or any of its sub-classes. - Can performance or memory usage be optimized?
- Could you maintain the code?
- Are the naming conventions clear or should some of the class and method names be changed?
- What would be the highest maintenance cost if you needed to maintain this code?
Development Environment
- C++ 20
- CMake
- GCC 12
- Ubuntu 22.04
- Visual Studio Code (vscode)
The Code
The GitHub repository for this code. It includes a CMakeLists.txt file if you want to build and run the test. For the exact files presented in this question you want the PreCoddeReview branch. Based on the reviews I am making changes which will be pushed to the master.
AbstractDictionary.h
#ifndef ABSTRACTDICTIONARY_H_
#define ABSTRACTDICTIONARY_H_
#include <algorithm>
#include <vector>
/*
* This abstract class provides a basic conversion of enums or integers to strings.
* More complex objects than strings are also supported, however, overrides will
* be necessary;
*
* Enums or integers should have an illegal value as 0. If not, then override the
* getIds method.
*/
template <typename dicID, typename dicName>
class DictionaryBase
{
public:
virtual dicID getIds(dicName itemName) noexcept
{
dicID foundID = static_cast<dicID>(0);
auto definition = std::find_if(searchTable.begin(), searchTable.end(),
[&itemName](struct DictType &dicItem) {return (dicItem.names == itemName);});
if (definition != searchTable.end())
{
foundID = definition->id;
}
return foundID;
};
virtual dicName getNames(dicID id) noexcept
{
dicName nameFound;
auto definition = std::find_if(searchTable.begin(), searchTable.end(),
[&id](struct DictType &dicItem) {return (dicItem.id == id);});
if (definition != searchTable.end())
{
nameFound = definition->names;
}
return nameFound;
}
virtual bool noOverRidesNeed() noexcept = 0;
protected:
struct DictType
{
dicID id;
dicName names;
};
std::vector<DictType> searchTable;
void addDefinition(dicID inId, dicName inName) noexcept
{
DictType newDef(inId, inName);
searchTable.push_back(newDef);
}
};
#endif // ABSTRACTDICTIONARY_H_#include <algorithm>
ColumnDictionary.cpp
#include "ColumnDictionary.h"
#include <string>
#include <vector>
ColumnDictionary::ColumnDictionary()
{
addDefinition(ColumnIds::DATE, "Date");
addDefinition(ColumnIds::TEMPERATURE, "Temperature");
addDefinition(ColumnIds::PUSLE_RATE, "Pulse Rate");
addDefinition(ColumnIds::RESPIRATION_RATE, "Respiration Rate");
addDefinition(ColumnIds::BLOOD_PRESSURE, "Blood Pressure");
addDefinition(ColumnIds::BLOOD_OXYGEN, "Blood Oxygen");
addDefinition(ColumnIds::WEIGHT, "Weight");
addDefinition(ColumnIds::BLOOD_SUGAR, "Blood Glucose");
addDefinition(ColumnIds::EXERCISE, "Exercise");
addDefinition(ColumnIds::SLEEP_HOURS, "Sleep Hours");
addDefinition(ColumnIds::SLEEP_INTERRUPTIONS, "Sleep Interruptions");
addDefinition(ColumnIds::BOWEL_MOVEMENTS, "Bowel Movements");
addDefinition(ColumnIds::NUTRITION_CALORIES, "Calories");
addDefinition(ColumnIds::NUTRITION_SATURATED_FAT, "Saturated Fat");
addDefinition(ColumnIds::NUTRITION_TRANS_FAT, "Trans Fat");
addDefinition(ColumnIds::NUTRITION_CHOLESTEROL, "Cholesterol");
addDefinition(ColumnIds::NUTRITION_SODIUM, "Sodium");
addDefinition(ColumnIds::NUTRITION_FIBER, "Fiber");
addDefinition(ColumnIds::NUTRITION_TOTAL_SUGARS, "Total Sugars");
addDefinition(ColumnIds::NUTRITION_PROTIEN, "Protien");
addDefinition(ColumnIds::NUTRITION_VITAMIN_D, "Vitamin D");
addDefinition(ColumnIds::NUTRITION_CALCIUM, "Calcium");
addDefinition(ColumnIds::NUTRITION_IRON, "Iron");
addDefinition(ColumnIds::NUTRITION_POTASSIUM, "Potassium");
addDefinition(ColumnIds::NOTES, "Notes");
}
ColumnDictionary.h
#ifndef COLUMNDICTIONARY_H_
#define COLUMNDICTIONARY_H_
#include "AbstractDictionary.h"
#include "columnidenum.h"
#include <string>
class ColumnDictionary : public DictionaryBase<ColumnIds, std::string>
{
public:
ColumnDictionary();
~ColumnDictionary() = default;
bool noOverRidesNeed() noexcept override { return true; }
};
#endif //COLUMNDICTIONARY_H_
columnidenum.h
#ifndef COLUMNIDENUM_H
#define COLUMNIDENUM_H
/*
* This enum is for use in selecting column names to display and the order of
* the comlumns in the table display.
*/
enum class ColumnIds
{
NO_COLUMN,
DATE,
// Vital Signs
TEMPERATURE,
PUSLE_RATE,
RESPIRATION_RATE,
BLOOD_PRESSURE,
BLOOD_OXYGEN,
WEIGHT,
BLOOD_SUGAR,
// Other things that might affect vital signs
EXERCISE,
SLEEP_HOURS,
SLEEP_INTERRUPTIONS,
BOWEL_MOVEMENTS,
// Nutritional Information
NUTRITION_CALORIES,
NUTRITION_SATURATED_FAT,
NUTRITION_TRANS_FAT,
NUTRITION_CHOLESTEROL,
NUTRITION_SODIUM,
NUTRITION_FIBER,
NUTRITION_TOTAL_SUGARS,
NUTRITION_PROTIEN,
NUTRITION_VITAMIN_D,
NUTRITION_CALCIUM,
NUTRITION_IRON,
NUTRITION_POTASSIUM,
NOTES,
LAST_COLUMN_ID
};
#endif // COLUMNIDENUM_H
TableDictionary.cpp
#include <string>
#include "TableDictionary.h"
#include <utility>
TableDictionary::TableDictionary()
{
addDefinition(TableIds::TEMPURATURE, {"Tempurature", "tmp"});
addDefinition(TableIds::REPIRATION_RATE, {"RepirationRate", "resp"});
addDefinition(TableIds::BLOOD_PRESSURE, {"BloodPressure", "bp"});;
addDefinition(TableIds::BLOOD_OXYGEN, {"BloodOxygen", "bo"});
addDefinition(TableIds::WEIGHT, {"Weight", "wt"});
addDefinition(TableIds::BLOOD_SUGAR, {"BloodSugar", "bg"});
addDefinition(TableIds::EXERCISE, {"Exercise", "exer"});
addDefinition(TableIds::SLEEP, {"Sleep", "slp"});
addDefinition(TableIds::BOWEL_MOVEMENTS, {"BowlMovements", "bm"});
addDefinition(TableIds::NUTRITION, {"Nutrition", "nut"});
addDefinition(TableIds::NOTES, {"Notes", "nts"});
}
std::string TableDictionary::getShortName(TableIds key) noexcept
{
return getNames(key).second;
}
std::string TableDictionary::getTableName(TableIds key) noexcept
{
return getNames(key).first;
}
TableIds TableDictionary::getId(std::string key) noexcept
{
TableIds id = TableIds::NO_TABLE;
auto definition = std::find_if(searchTable.begin(), searchTable.end(),
[&key](struct DictType &dicItem) {
return (dicItem.names.first == key ||
dicItem.names.second == key);
});
if (definition != searchTable.end())
{
id = definition->id;
}
return id;
}
TableDictionary.h
#ifndef TABLEDICTIONARY_H
#define TABLEDICTIONARY_H
#include "AbstractDictionary.h"
#include <string>
#include "tableidenum.h"
#include <utility>
class TableDictionary : public DictionaryBase<TableIds, std::pair<std::string, std::string>>
{
public:
TableDictionary();
~TableDictionary() = default;
bool noOverRidesNeed() noexcept override { return false; }
std::string getShortName(TableIds id) noexcept;
std::string getTableName(TableIds key) noexcept;
TableIds getId(std::string name) noexcept;
};
#endif
tableidenum.h
#ifndef TABLEIDENUM_H
#define TABLEIDENUM_H
enum class TableIds
{
NO_TABLE,
TEMPURATURE,
REPIRATION_RATE,
BLOOD_PRESSURE, // Currently Blood Pressure and Pulse Rate
BLOOD_OXYGEN,
WEIGHT,
BLOOD_SUGAR,
EXERCISE,
SLEEP,
BOWEL_MOVEMENTS,
NUTRITION,
NOTES,
LAST_TABLE_ID
};
#endif // TABLEIDENUM_H
TestDictionary.cpp
#include "AbstractDictionary.h"
#include "ColumnDictionary.h"
#include "columnidenum.h"
#include <iostream>
#include <string>
#include "TableDictionary.h"
#include <vector>
/*
* Testing the basic implementation.
*/
enum class BaseTestEnum
{
BASETEST_INVALID,
BASETEST_1,
BASETEST_2,
BASETEST_3,
BASETEST_LAST
};
class BaseTestClass : public DictionaryBase<BaseTestEnum, std::string>
{
public:
BaseTestClass()
{
addDefinition(BaseTestEnum::BASETEST_1, "Base Test 1");
addDefinition(BaseTestEnum::BASETEST_2, "Base Test 2");
addDefinition(BaseTestEnum::BASETEST_3, "Base Test 3");
}
~BaseTestClass() = default;
bool noOverRidesNeed() noexcept override { return true; }
};
static bool BaseTestIdToName(BaseTestEnum input, std::string expectedOutput)
{
BaseTestClass underTest;
std::string actualOutPut = underTest.getNames(input);
if (!expectedOutput.compare(actualOutPut) == 0)
{
std::cerr << "\tBase ID to Name Test Failed " << expectedOutput << "\n";
return false;
}
return true;
}
static bool BaseTestNameToID(std::string input, BaseTestEnum expectedOutput)
{
BaseTestClass underTest;
BaseTestEnum actualOutPut = underTest.getIds(input);
if (expectedOutput != actualOutPut)
{
std::cerr << "\tBase Name to ID Test Failed " << input << "\n";
return false;
}
return true;
}
static bool basicTestIdsDictionary()
{
bool testPassed = true;
std::cout << "\nTesting base implementation\n";
if (!BaseTestIdToName(BaseTestEnum::BASETEST_1, "Base Test 1"))
{
return false;
}
if (!BaseTestIdToName(BaseTestEnum::BASETEST_2, "Base Test 2"))
{
return false;
}
if (!BaseTestIdToName(BaseTestEnum::BASETEST_3, "Base Test 3"))
{
return false;
}
if (!BaseTestNameToID("Base Test 1", BaseTestEnum::BASETEST_1))
{
return false;
}
if (!BaseTestNameToID("Base Test 2", BaseTestEnum::BASETEST_2))
{
return false;
}
if (!BaseTestNameToID("Base Test 3", BaseTestEnum::BASETEST_3))
{
return false;
}
// Negative Test Path
if (!BaseTestIdToName(static_cast<BaseTestEnum>(217), ""))
{
return false;
}
if (!BaseTestNameToID("Name Not Found", BaseTestEnum::BASETEST_INVALID))
{
return false;
}
return testPassed;
}
/*
* Testing TableDictionary class which adds some extenstions to the base clase.
*/
static bool TableTestIdToName(TableIds input, std::pair<std::string, std::string> expectedOutput)
{
TableDictionary underTest;
std::pair<std::string, std::string> actualOutPut(underTest.getNames(input));
if (expectedOutput.first.compare(actualOutPut.first) != 0 ||
expectedOutput.second.compare(actualOutPut.second) != 0)
{
std::cerr << "\tTable ID to Name Test Failed (" << expectedOutput.first << " " << expectedOutput.second << ")\n";
return false;
}
return true;
}
static bool TableTestNameToID(std::pair<std::string, std::string> input, TableIds expectedOutput)
{
TableDictionary underTest;
TableIds actualOutPut = underTest.getIds(input);
if (expectedOutput != actualOutPut)
{
std::cerr << "\tTable Name to ID Test Failed (" << input.first << " " << input.second << ")\n";
return false;
}
return true;
}
static bool TableTestNameToID(std::string input, TableIds expectedOutput)
{
TableDictionary underTest;
TableIds actualOutPut = underTest.getId(input);
if (expectedOutput != actualOutPut)
{
std::cerr << "\tSingle Table Name to ID Test Failed " << input << "\n";
return false;
}
return true;
}
typedef struct TableIDTestData
{
TableIds testID;
std::pair<std::string, std::string> testNames;
} TableIDTestData;
static std::vector<TableIDTestData> TTestData =
{
{TableIds::TEMPURATURE, {"Tempurature", "tmp"}},
{TableIds::REPIRATION_RATE, {"RepirationRate", "resp"}},
{TableIds::BLOOD_PRESSURE, {"BloodPressure", "bp"}},
{TableIds::BLOOD_OXYGEN, {"BloodOxygen", "bo"}},
{TableIds::WEIGHT, {"Weight", "wt"}},
{TableIds::BLOOD_SUGAR, {"BloodSugar", "bg"}},
{TableIds::EXERCISE, {"Exercise", "exer"}},
{TableIds::SLEEP, {"Sleep", "slp"}},
{TableIds::BOWEL_MOVEMENTS, {"BowlMovements", "bm"}},
{TableIds::NUTRITION, {"Nutrition", "nut"}},
{TableIds::NOTES, {"Notes", "nts"}},
};
static bool testTableIdsDictionary()
{
bool testPassed = true;
std::cout << "\nTesting TableDictionary\n";
for (auto testCandidate: TTestData)
{
if (!TableTestIdToName(testCandidate.testID, testCandidate.testNames))
{
return false;
}
if (!TableTestNameToID(testCandidate.testNames, testCandidate.testID))
{
return false;
}
if (!TableTestNameToID(testCandidate.testNames.first, testCandidate.testID))
{
return false;
}
if (!TableTestNameToID(testCandidate.testNames.second, testCandidate.testID))
{
return false;
}
}
// Negative Test Path
if (!TableTestIdToName(static_cast<TableIds>(217), {"",""}))
{
return false;
}
if (!TableTestNameToID(std::pair("Name Not Found", "bad"), TableIds::NO_TABLE))
{
return false;
}
if (!TableTestNameToID("Name Not Found", TableIds::NO_TABLE))
{
return false;
}
return testPassed;
}
/*
* Testing the ColumnDictionary class, which just uses the base class.
*/
static bool ColumnTestIdToName(ColumnIds input, std::string expectedOutput)
{
ColumnDictionary underTest;
std::string actualOutPut = underTest.getNames(input);
if (!expectedOutput.compare(actualOutPut) == 0)
{
std::cerr << "Column ID to Name Test Failed " << expectedOutput << "\n";
return false;
}
return true;
}
static bool ColumnTestNameToID(std::string input, ColumnIds expectedOutput)
{
ColumnDictionary underTest;
ColumnIds actualOutPut = underTest.getIds(input);
if (expectedOutput != actualOutPut)
{
std::cerr << "\tColumn Name to ID Test Failed " << input << "\n";
return false;
}
return true;
}
typedef struct CTestData
{
ColumnIds testId;
std::string testName;
} CTestData;
static std::vector<CTestData> positiveColumnTestData =
{
{ColumnIds::DATE, "Date"},
{ColumnIds::TEMPERATURE, "Temperature"},
{ColumnIds::PUSLE_RATE, "Pulse Rate"},
{ColumnIds::RESPIRATION_RATE, "Respiration Rate"},
{ColumnIds::BLOOD_PRESSURE, "Blood Pressure"},
{ColumnIds::BLOOD_OXYGEN, "Blood Oxygen"},
{ColumnIds::WEIGHT, "Weight"},
{ColumnIds::BLOOD_SUGAR, "Blood Glucose"},
{ColumnIds::EXERCISE, "Exercise"},
{ColumnIds::SLEEP_HOURS, "Sleep Hours"},
{ColumnIds::SLEEP_INTERRUPTIONS, "Sleep Interruptions"},
{ColumnIds::BOWEL_MOVEMENTS, "Bowel Movements"},
{ColumnIds::NUTRITION_CALORIES, "Calories"},
{ColumnIds::NUTRITION_SATURATED_FAT, "Saturated Fat"},
{ColumnIds::NUTRITION_TRANS_FAT, "Trans Fat"},
{ColumnIds::NUTRITION_CHOLESTEROL, "Cholesterol"},
{ColumnIds::NUTRITION_SODIUM, "Sodium"},
{ColumnIds::NUTRITION_FIBER, "Fiber"},
{ColumnIds::NUTRITION_TOTAL_SUGARS, "Total Sugars"},
{ColumnIds::NUTRITION_PROTIEN, "Protien"},
{ColumnIds::NUTRITION_VITAMIN_D, "Vitamin D"},
{ColumnIds::NUTRITION_CALCIUM, "Calcium"},
{ColumnIds::NUTRITION_IRON, "Iron"},
{ColumnIds::NUTRITION_POTASSIUM, "Potassium"},
{ColumnIds::NOTES, "Notes"}
};
static bool testColumnIdsDictionary()
{
bool testPassed = true;
std::cout << "\nTesting ColumnDictionary\n";
for (auto testCandidate: positiveColumnTestData)
{
if (!ColumnTestIdToName(testCandidate.testId, testCandidate.testName))
{
return false;
}
if (!ColumnTestNameToID(testCandidate.testName, testCandidate.testId))
{
return false;
}
}
// Negative Test Path
if (!ColumnTestIdToName(static_cast<ColumnIds>(217), ""))
{
return false;
}
if (!ColumnTestNameToID("Name Not Found", ColumnIds::NO_COLUMN))
{
return false;
}
return testPassed;
}
int main()
{
int exitStatus = EXIT_SUCCESS;
if (!basicTestIdsDictionary())
{
exitStatus = EXIT_FAILURE;
}
if (!testColumnIdsDictionary())
{
exitStatus = EXIT_FAILURE;
}
if (!testTableIdsDictionary())
{
exitStatus = EXIT_FAILURE;
}
std::cout << (exitStatus == EXIT_FAILURE ? "Tests Failed\n\n" : "All Tests Passed\n\n");
return exitStatus;
}
cat src/* > CR_Question.txt. I meant to change the order, but didn't get to it. \$\endgroup\$