[Bug] Memory leaks in DataFrame class, etc.

[gbowne1]

There are some issues with the DataFrame class:

• Memory Leak in operator[] Overload
• The sizeRows function returns the number of columns (keys in the data map), which I think is a bit misleading.
• The load and save functions return an integer status code. I think it would be better to throw an exception
• The operator[] that returns std::vectorstd::string & does not check if the key exists in the data map and might have some potential for out of bounds access.
• print function could be a bit more efficient. The print function calculates the line width by iterating over the columnSizes vector multiple times. This could be optimized by calculating the total width in a single pass.
• std::endl probably should be \n
• the CSV load and save functions could be optimized so it would work if the CSV's are not very well formed or missing things.

[gbowne1]

The memory leak could be fixed with

std::map<std::string, std::string&> DataFrame::operator[](size_t index) {
    std::map<std::string, std::string&> series;
    for (auto& col : this->data) {
        series[col.first] = col.second.at(index);
    }
    return series;
}

[andrewlod]

Hey!

Are you sure it is a memory leak? As far as I remember, when I designed the DataFrame, I did not allocate any memory (and when I do, it is via smart pointers). I have tried returning a reference instead of a pointer, but it returned some error I don't remember (I have gone through this though: https://stackoverflow.com/questions/1543193/why-cant-i-store-references-in-a-stdmap-in-c). Also, it is a non-owning pointer, meant to be used as a reference.

Did you try running it with Valgrind and see if it reports anything?

I will address some of these issues and fix them when I get some time!

[andrewlod]

std::endl probably should be \n

The reason I used std::endl instead of \n was because it flushes the output buffer, while \n does not. (https://www.geeksforgeeks.org/endl-vs-n-in-cpp/).

[gbowne1]

I really need to get into Valgrind testing this and I haven't so far yet. Fair enough for using endl for that. Just some thoughts I had. Just did some objective debugging on this looking for small things that could cause issues. My big problem is with the operator[].. just needs more debugging and testing.

Do the sizeRows and sizeCols seem inconsistent?

I think the code overall is pretty good.