0

I have an ordered array list. And in my resize function I create a new array and assign it the values of the old array and then I delete the old array using delete[] arrayname;. This causes an error at run-time whenever the resize function comes into play. dbgheap.c is called. Has anyone ever seen this before?

Here is my code:

//--------------------------------------------------------------------------------------------
//  Name:           OrderedArray.h.
//  Description:    Header file for the use in OrderedArray.cpp.
//--------------------------------------------------------------------------------------------
#ifndef ORDEREDARRAY_H
#define ORDEREDARRAY_H
#include <iostream>

using namespace std;
//--------------------------------------------------------------------------------------------
//  Name:           template <class Datatype>   
//  Description:        
//--------------------------------------------------------------------------------------------
template <class Datatype>
//--------------------------------------------------------------------------------------------
//  Class: OrderedArray.            
//--------------------------------------------------------------------------------------------
class OrderedArray
{
//--------------------------------------------------------------------------------------------
//  Member Variables.           
//--------------------------------------------------------------------------------------------
private:
Datatype* m_array;
int size;
int g_size;
int num_elements;   //Counter for the number of elements in the Array.

void Resize(int p_size)//resizes the array to the size of p_size
    {
        cout << "Did i get this far ";
        if(p_size < 0)//checks if new size is less than 0
        {
            cout << "ERROR! Size of an array can not be less than 0!" << endl;
        }
        else//else its ok to continue
        {
            Datatype* newArray = new Datatype[p_size];//creates a pointer newArray that points at a new array
            if(newArray == 0)
            {
                return;
            }
            cout << "Did i get this far ";
            int min;

            if(p_size < size)//checks the if the new array is smaller than the old one
                min = p_size;
            else//else its going to be bigger
                min = size;
            cout << "Did i get this far ";
            int index;
            int temp = num_elements;//puts num_elements into a temporary variable called temp
            num_elements = 0;//num_elements is set to 0
            for(index = 0; index < min; index++)
            {
                newArray[index] = m_array[index];//places everything from the old array into the new array that will fit.
                if(num_elements < temp)//if the num_elements is less than temp(the original num_elements)
                {
                    num_elements++;//increment num_elements. This will keep incrementing to create the new num_elements based the number of elements cut off in the resize
                }
            }
            size = p_size;//sets the old size to be equal to the new size
            cout << "Did i get this far ";
            if(m_array != 0)
            {
            cout << "\nI am just about to delete ";
            delete[] m_array;//deletes the old array
            }
            m_array = newArray;//makes m_array point at the new array
            newArray = 0;//makes newArray a null pointer
        }
    }
//---------------------------------------------------------------------------------------
// Name:             Push
// Description:      
//---------------------------------------------------------------------------------------
void push(Datatype p_item)
{
    if(num_elements == size)//checks if the array is full and needs to be resized
    {
        Resize(size + g_size);//calls the resize function
    }

    int pos = num_elements;
    for(int x=0;x<num_elements;x++)
    {
        if(p_item < m_array[x])
        {
        pos=x;
        }
    }

    //loops through the array from high to low moving all values to the right
    //to make space for the passed in value until it gets to the right place
    for(int index = num_elements; index >= pos; index--)
    {
        m_array[index] = m_array[index-1];//moves the values to the right
    }
        m_array[pos] = p_item;//the passed in value is positioned into its ordered position
        num_elements++;

    cout<< "Num Elements " << num_elements;
    cout<< "Size " <<size;
}

    //--------------------------------------------------------------------------------------------
    //  Name:           Constructor.
    //  Description:    Constructs the Array.
    //--------------------------------------------------------------------------------------------
    OrderedArray(int p_size, int grow_size)
    {
        //Sets the Array size.
        m_array = new Datatype[p_size,grow_size];   
        size = p_size;
        g_size = grow_size; 
        //How many elements are in the Array.
    num_elements = 0;               
}

//size and g_size are given its value by the user at the start of the program.

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Becca
  • 117
  • 1
  • 12
  • You're likely overwriting the bounds of your array, and the checking isn't performed until releasing the memory to the heap. You should show how you're populating the array, and try to reduce this to a http://sscce.org/ – Chad Mar 12 '13 at 20:45
  • Ok, Im populating the array with an ordered push at the users discretion... Il edit the push function into my code :) – Becca Mar 12 '13 at 20:46
  • And when debugging runtime errors - use a debugger (gdb for instance). It just makes everything so much easier. – sonicwave Mar 12 '13 at 20:50
  • @Becca: How are you using your class? – AnT stands with Russia Mar 12 '13 at 20:57
  • @AndreyT. I have my class in a header file and the main is in a .cpp. – Becca Mar 12 '13 at 20:58
  • @Becca: I mean the actual code. How are you declaring and initializing the object? What methods are you calling? – AnT stands with Russia Mar 12 '13 at 20:58
  • Im using a template of integers. And im initializing the array like OrderedArray Array(array_size,growth_size); The array_size and growth_size are chosen by the user at the start of the program. Here is my constructor. – Becca Mar 12 '13 at 21:04
  • 3
    `m_array = new Datatype[p_size,grow_size];` !!! – paddy Mar 12 '13 at 21:17

2 Answers2

4

There may be some other issue here, but the most obvious thing I can see is this:

for(int index = num_elements; index >= pos; index--)
{
    m_array[index] = m_array[index-1];
}

If pos happens to be zero, you will eventually do this:

    m_array[0] = m_array[-1];

This problem will show immediately (when num_elements is zero - you haven't shown your constructor, but I do hope that you initialised everything).

Changing >= to > in the loop may solve all your troubles.

Conceptually, you ought to agree with this logic. There is no point moving the item before m_array[pos] forward to m_array[pos] when you are just about to replace it with the new item.


[edit] Now that you have posted your constructor:

m_array = new Datatype[p_size,grow_size];   

This will initialise your array with the size grow_size instead of p_size, because of how the comma operator works. Read this: How does the Comma Operator work

Please do this instead:

m_array = new Datatype[p_size];
Community
  • 1
  • 1
paddy
  • 60,864
  • 6
  • 61
  • 103
  • Also, isn't `m_array[index]` one past the end on the first trip through the loop? – Jesse Good Mar 12 '13 at 21:04
  • I did that... unfortunatly no joy... Yes everything is inistialised in the constructor. I will edit it in... – Becca Mar 12 '13 at 21:08
  • @JesseGood No, look at the top of the `push` function. The array should be resized. – paddy Mar 12 '13 at 21:08
  • @Becca I should give you a warning that it's not a good idea to resize only when `num_elements == size`. This code could be easily broken if you are not careful. It's safer to test `num_elements >= size`. – paddy Mar 12 '13 at 21:10
  • @paddy K i appreciate the advice... I changed that to greater or equals :) – Becca Mar 12 '13 at 21:12
  • Just saw your constructor. Why on earth are you using the comma operator in your call `new Datatype[p_size,grow_size]`? Please just call it with `p_size`. I think the comma operator in this case has caused you to initialise the array size to `grow_size`. That would back up my previous comment about safety. – paddy Mar 12 '13 at 21:15
  • I need to do it this way so that i can ask the user to decide the array size and how many blocks are allocated when a resize initiates... – Becca Mar 12 '13 at 21:18
  • You are welcome to ignore my advice, but the `new` operator does not work that way. I'm not saying that you change your class, only that you use `new` the way it was intended. Using a comma here does not do what you think it does. Read this: http://stackoverflow.com/questions/54142/how-does-the-comma-operator-work – paddy Mar 12 '13 at 21:21
  • I just took the second parameter out like you said and it fixed a number of problems i was having :S. Thank you :) lol – Becca Mar 12 '13 at 21:24
1

Your code m_array = new Datatype[p_size,grow_size]; in the constructor should only take one parameter which is the size of the array.

Change to this: m_array = new Datatype[p_size];

Pendo826
  • 1,002
  • 18
  • 47
  • 1
    Not to gripe, but why did this become the accepted answer when it was posted after I had determined that to be the problem and my answer had been accepted? – paddy Mar 12 '13 at 22:42
  • Because you have way more reputation than Pendo. lol. Like me, the OP probably like reading the shorter answer first. So, I'm assuming he saw both posts at the same time and chose to read the shorter answer first. – ArgumentNullException Apr 15 '13 at 22:21