0

I've been beating my head against this one for awhile. In the deconstructor of my class, I have a for loop that is supposed to iterate through an array of objects and delete them. When I try though, I get a read access violation. The attached code is supposed to read info from two documents and use that to create Country objects.

#include "pch.h"
#include "CountryCatalogue.h"
#include "Country.h"
#include <iterator>
#include <map>
//imports for reading the files
#include <iostream>
#include <fstream>

CountryCatalogue::CountryCatalogue()
{
    _maxSize = 10;
    _curSize = 0;
    _catalogue = new Country*[_maxSize];
}

CountryCatalogue::CountryCatalogue(std::string continentFileName, std::string countryFileName)
{

//block that opens the files and checks to make sure they can be read
//open up the files
std::ifstream inFile1;
std::ifstream inFile2;

//opening both text files and ensuring that the file is readable to the program
inFile1.open(continentFileName);
if (!inFile1) {
    std::cout << "Unable to open file";
    exit(1); // terminate with error
}

inFile2.open(countryFileName);
if (!inFile2) {
    std::cout << "Unable to open file";
    exit(1); // terminate with error
}

// read the continet file
// while there is still stuff to read in the file
std::string str;

while (!inFile1.eof())
{
    std::string Country, Cont;

    //reading lines from file and assigning to variables
    std::getline(inFile1, Country);
    std::getline(inFile1, Cont);

    //mapping to variables read from file
    _countryContinent.insert(std::pair<std::string, std::string>(Country, Cont));
    _curSize++;
}

//closing file after use
inFile1.close();

//creating array
_catalogue = new Country*[_curSize+2];

//resetting size to zero for later itteration
_curSize = 0;

// read the country file
// while there is still stuff to read in the file
while (!inFile2.eof())
{
    std::string name, POP, AREA;
    int pop;
    double area = 0.0;

    std::getline(inFile2, name);
    std::getline(inFile2, POP);
    std::getline(inFile2, AREA);

    if (!POP.empty() && POP[POP.length() - 1] == '\n') {
        POP.erase(POP.length() - 1);
    }
    if (!AREA.empty() && AREA[AREA.length() - 1] == '\n') {
        AREA.erase(AREA.length() - 1);
    }

    pop = std::stoi(POP);
    area = std::stod(AREA);

    //creating iterator to search through mapped values
    std::map<std::string, std::string>::iterator it;
    it = _countryContinent.find(name);

    //creating empty string variable to store continent
    std::string cont;

    //using value found by iterator to make continent string
    //ensuring value isn't the end valueof the map
    if (it != _countryContinent.end()){
        cont = it->second;
    }

    //std::cout << name << pop << area << cont << std::endl;

    // add the country to the catalogue
    addCountry(name, pop, area, cont);

    }
}

CountryCatalogue::~CountryCatalogue() {

    /*for (int i = 0; i < _curSize; i++){
        delete _catalogue[i];
        std::cout << "deleted" << i << std::endl;
    }*/
    delete[] _catalogue;
}

void CountryCatalogue::addCountry(std::string name, int pop, double area,     std::string cont) {

    //std::cout << name << pop << area << cont << std::endl;
    //std::cout << _curSize << std::endl;

    Country* toAdd = new Country(name, pop, area, cont);
    if (_curSize == _maxSize) {
        expandCapacity();
    }

    //adding country object to array
    _catalogue[_curSize] = toAdd;

    //adding to _curSize for next iteration
    _curSize++;
}


void CountryCatalogue::printCountryCatalogue() {

    std::string s;


    /*for (int i = 0; i < _curSize; i++) {
        s += _catalogue[i]->to_string() + "\n";
    }*/ 

    std::cout << _curSize << std::endl;
}

void CountryCatalogue::expandCapacity() {

    //doubling array size
    _maxSize = _maxSize * 2;

    //creating pointer to new array of new size
    Country** newCatalogue = new Country*[_maxSize];

    //copying old array into new
    for (int i = 0; i < _curSize; i++) {
        newCatalogue[i] = _catalogue[i];
    }

    //deleting old array
    delete[] _catalogue;

    //making _catalogue point to newCatalogue
    _catalogue = newCatalogue;
}

UPDATE: What my code is supposed to do is get information from text files and create objects using that data. I am required to use an array instead of a vector. The code runs fine and I can create the country object. The issue is that I cannot add the created object to the _catalogue array, as I cannot delete it afterwards. When I attempt to iterate through the array, I receive a message saying Heap Corruption was detected.

Jake
  • 3
  • 3
  • What is `_countryContinent`? Why do you have `_curSize++;` just after `_countryContinent.insert` ? – Support Ukraine Feb 07 '19 at 05:56
  • `_countryContinent` is just a map between a country and it's continent. I use `_curSize++` after that as a way of counting how many countries there are to make the dynamic array. I added the plus two after to see if that would get rid of the read access voilation – Jake Feb 07 '19 at 06:08
  • 2
    If you are programming C++, you should be using `std::vector`. Then, you don't have a problem. – Cody Gray - on strike Feb 07 '19 at 06:13
  • I'm not allowed to use vectors in this project as it's for school – Jake Feb 07 '19 at 06:16
  • 1
    Unrelated (probably): Reading [Why is iostream::eof inside a loop condition considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) will help you defeat a bug you likely haven't found yet. – user4581301 Feb 07 '19 at 06:17
  • 1
    A debugger should help you isolate the problem. The debugger will halt the program as soon as the error occurs allowing you to inspect the program's backtrace and the variables in play when the program crashed. If this doesn't reveal the problem outright, it will give you many avenues of exploration. Right now your code is both too long and incomplete. This makes it much harder than necessary to help you isolate the problem. For example, I cannot simply drop your code into my debugger. Please read [mcve] for help on isolating the problem. – user4581301 Feb 07 '19 at 06:24
  • @user4581301 "The debugger will halt the program as soon as the error occurs". No it will not in general, and certainly will not in this case. – n. m. could be an AI Feb 07 '19 at 06:45
  • Please post a [mcve]. Due to temporary technical issues we only can find problemns in code we can see. Sorry for the inconvenience. – n. m. could be an AI Feb 07 '19 at 06:49

2 Answers2

1

You created _catalogue as a dynamic array.

To release the memory allocated for arrays of elements using new TYPE[SIZE] the syntax is:

delete[] _catalogue; 

Loop is Needed for deleting memory allocated for Matrix elements. For example

int matrix = new int[rows][cols];
for (int i = 0; i < rows; ++i)
    delete [] matrix[i];   

The array is deleted row by row.

user4581301
  • 33,082
  • 7
  • 33
  • 54
Akash das
  • 371
  • 4
  • 9
  • I try and do that in the decondtructor, but I get an error when I try I get a null pointer exception – Jake Feb 07 '19 at 06:18
1

Your problem is due to this line

_catalogue = new Country*[_curSize+2];

in the second constructor. You have forgotten to update _maxSize so you have a mismatch between _maxSize and the real allocated amount of memory.

Try:

_maxSize = _curSize+2;
_catalogue = new Country*[_maxSize];
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
  • I've determined that the line you're referencing is actually causing a memory leak, but I'm not sure how to fix it. – Jake Feb 07 '19 at 09:08
  • @Jake I'm not sure what you mean by that comment. The fix is given in my answer, i.e. make sure to initialize `_maxSize` – Support Ukraine Feb 07 '19 at 09:13
  • I've created the array using `_maxSize` but I'm getting a memory leak flag on the line declaring the array. Everything online says to make sure that I delete the array, but I already do in the deconstructor – Jake Feb 07 '19 at 09:19
  • @Jake I'm still confused about your comment. Are you saying that you have updated your code with my fix and the code still fails? – Support Ukraine Feb 07 '19 at 09:24
  • Sorry for the confusion, your solution worked. Thanks! – Jake Feb 07 '19 at 09:43