2

Im trying to write a class that stores an id and a value in an container class. Im using an nested class as my data structure. When im compiling the code sometimes it prints perfectly, sometimes it prints nothing and sometimes it prints half of the data then stops. When i debug the code the same weird behavior occours, when it fails during debug it throws an error "Map.exe has triggered a breakpoint.", the Error occours in the print method when im using cout.

cmap.h

#pragma once


class CMap
{
public:
    CMap();
    ~CMap();
    CMap& Add(int id, int value);
    void print() const;


private:

    class container
    {
    public:
        ~container();
        int container_id = 0;
       int container_value = 0; 
    };
    container* p_komp_;
    int dim_ = -1;

    void resize();
};

cmap.cpp

#include "cmap.h"
#include <iostream>
using namespace std;



CMap::CMap()
{
    p_komp_ = new container[0];
}


CMap::~CMap()
{
    p_komp_ = nullptr;
    cout << "destroy cmap";
}


CMap& CMap::Add(int id, int value)
{
    resize();
    p_komp_[dim_].container_id = id;
    p_komp_[dim_].container_value = value;
    return *this;
}

void CMap::resize()
{
    container* temp_array = new container[++dim_];

    if (dim_ == 0)
    {
        temp_array[0].container_id = p_komp_[0].container_id;
        temp_array[0].container_value = p_komp_[0].container_value;
    }
    for (unsigned i = 0; i < dim_; i++)
    {
        temp_array[i].container_id = p_komp_[i].container_id;
        temp_array[i].container_value = p_komp_[i].container_value;
    }

    p_komp_ = temp_array;

}

void CMap::print() const
{

    for (unsigned i = 0; i <= dim_; i++)
    {    

        cout << p_komp_[i].container_id;
        cout << p_komp_[i].container_value;

    }
}


CMap::container::~container()
{
    cout << "destruct container";

}

Map.cpp

#include "cmap.h"
#include <iostream>

using namespace std;

    void main(void)
    {

        CMap m2;
        m2.Add(1, 7);
        m2.Add(3, 5);
        m2.print();
    }
maxiangelo
  • 125
  • 12
  • 8
    What do you think `new container[0]` does? – scohe001 Jun 12 '18 at 17:30
  • 3
    C++ is not a garbage-collecting language, `p_komp_ = nullptr` in your destructor will not free the possible memory you might have allocated. – Some programmer dude Jun 12 '18 at 17:31
  • It creates an array with the size of one. – maxiangelo Jun 12 '18 at 17:32
  • What do you think `new container[10]` does? Does this create 11 containers? – MPops Jun 12 '18 at 17:34
  • when i tried to `delete[] p_komp_` in the destructor i got a heap corruption – maxiangelo Jun 12 '18 at 17:35
  • Why don't you just use `std::vector`? Is this a class exercise requiring you to implement your own vector-like class? – Barmar Jun 12 '18 at 17:50
  • it kind of is yes, i have more than a week to go but im trying to finish early – maxiangelo Jun 12 '18 at 17:54
  • 1
    That is a good plan @maxiangelo . Getting a dynamic array working is a lot trickier than people expect. I recommend a read-through of [The rule of three/five/zero](http://en.cppreference.com/w/cpp/language/rule_of_three) because this will get you ahead of a future bug you will encounter. You need to take the Rules of Three and Five into account any time you are copying or moving an object that contains a pointer or any other raw resource that requires special management. – user4581301 Jun 12 '18 at 18:04
  • [Here is a discussion on what happens with a zero-size dynamic array](https://stackoverflow.com/questions/1087042/c-new-int0-will-it-allocate-memory). You cannot use this array for much of anything. The heap corruption is likely the result of some implementation defined book-keeping information stored after your allocation. You ask for zero, but the runtime allocates zero plus storage for some information used to when you `delete[]` the allocation. When you write into `p_komp_[0]`, you write over this information and `delete[]` can no longer function. – user4581301 Jun 12 '18 at 18:17

1 Answers1

7

These two things are a possible reason for your problem:

int dim_ = -1;

and

container* temp_array = new container[++dim_];

When you allocate, you increase dim_ from -1 to 0. That is you create a zero-sized "array", where every indexing into it will be out of bounds and lead to undefined behavior.


You also have memory leaks since you never delete[] what you new[]. I didn't look for more problems, but there probably a more.

And an "array" (created at compile-time or through new[]) will have indexes from 0 to size - 1 (inclusive). You seem to think that the "size" you provide is the top index. It's not, it's the number of elements.

It seems to me that you might need to take a few steps back, get a couple of good books to read, and almost start over.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621