Reputation: 1858
I am trying to expand the functionality of a class template I created. Previously it allowed you to use key-value pairs of any type but only if you knew the size of the arrays at compile time. It looked like this:
template <typename K, typename V, int N>
class KVList {
size_t arraySize;
size_t numberOfElements;
K keys[N];
V values[N];
public:
KVList() : arraySize(N), numberOfElements(0) { }
// More member functions
}
I wanted to be able to use this for a dynamic number of elements decided at run-time, so I changed the code to this:
template <typename K, typename V>
class KVList {
size_t arraySize;
size_t numberOfElements;
K* keys;
V* values;
public:
KVList(size_t size) : numberOfElements(0) {
arraySize = size;
keys = new K[size];
values = new V[size];
}
~KVList() {
delete[] keys;
keys = nullptr;
delete[] values;
values = nullptr;
}
// More member functions
}
The new constructor has one parameter which is the size that will be used for the KVList. It still starts the numberOfElements at 0 because both of these uses would start the KVList empty, but it does set arraySize to the value of the size parameter. Then it dynamically allocated memory for the arrays of keys and values. An added destructor deallocates the memory for these arrays and then sets them to nullptr.
This compiles and runs, but it only stores the first key and first value I try to add to it. There is a member function in both that adds a key-value pair to the arrays. I tested this with the Visual Studio 2015 debugger and noticed it storing the first key-value pair fine, and then it attempts to store the next key-value pair in the next index, but the data goes no where. And the debugger only shows one slot in each array. When I attempt to cout the data I thought I stored at that second index, I get a very small number (float data type was trying to be stored), not the data I was trying to store.
I understand it might be worth using the vectors to accomplish this. However, this is an expansion on an assignment I completed in my C++ class in school and my goal with doing this was to try to get it done, and understand what might cause issues doing it this way, since this is the obvious way to me with the knowledge I have so far.
EDIT: Code used to add a key-value pair:
// Adds a new element to the list if room exists and returns a reference to the current object, does nothing if no room exists
KVList& add(const K& key, const V& value) {
if (numberOfElements < arraySize) {
keys[numberOfElements] = key;
values[numberOfElements] = value;
numberOfElements++;
}
return *this;
}
EDIT: Code that calls add():
// Temp strings for parts of a grade record
string studentNumber, grade;
// Get each part of the grade record
getline(fin, studentNumber, subGradeDelim); // subGradeDelim is a char whose value is ' '
getline(fin, grade, gradeDelim); // gradeDelim is a char whose value is '\n'
// Attempt to parse and store the data from the temp strings
try {
data.add(stoi(studentNumber), stof(grade)); // data is a KVList<size_t, float> attribute
}
catch (...) {
// Temporary safeguard, will implement throwing later
data.add(0u, -1);
}
Code used to test displaying the info:
void Grades::displayGrades(ostream& os) const {
// Just doing first two as test
os << data.value(0) << std::endl;
os << data.value(1);
}
Code in main cpp file used for testing:
Grades grades("w6.dat");
grades.displayGrades(cout);
Contents of w6.dat:
1022342 67.4
1024567 73.5
2031456 79.3
6032144 53.5
1053250 92.1
3026721 86.5
7420134 62.3
9762314 58.7
6521045 34.6
Output:
67.4
-1.9984e+18
Upvotes: 0
Views: 195
Reputation: 17704
The problem (or at least one of them) is with this line from your pastebin:
data = KVList<size_t, float>(records);
This seemingly innocent line is doing a lot. Because data already exists, being default constructed the instance that you entered the body of the Grades constructor, this will do three things:
You may be thinking: what copy assignment operator, I never wrote one. Well, the compiler generates it for you automatically. Actually, in C++11, generating a copy assignment operator automatically with an explicit destructor (as you have) is deprecated; but it's still there.
The problem is that the compiler generated copy assignment operator does not work well for you. All your member variables are trivial types: integers and pointers. So they just copied over. This means that after step 2, the class has just been copied over in the most obvious way. That, in turn, means that for a brief instance, there is an object on the left and right, that both have pointers pointing to the same place in memory. When step 3 fires, the right hand object actually goes ahead and deletes the memory. So data is left with pointers pointing to random junk memory. Writing to this random memory is undefined behavior, so your program may do (not necessarily deterministic) strange things.
There are (to be honest) many issues with how your explicit resource managing class is written, too many to be covered here. I think that in Accelerated C+, a really excellent book, it will walk you through these issues, and there is an entire chapter covering every single detail of how to properly write such a class.
Upvotes: 1