Resizable Array with pointers

Question

My program works but my professor says that my code is incorrect but stated that he will get to why in the fall term... What is he talking about? perhaps something is improper? Even if you are incorrect I would appreciate picking your brain :)

void CResizableArray::SetSize( int intNewSize )
{
    int intIndex = 0;

    if( intNewSize < 0 ) intNewSize = 0;
    if( intNewSize > intMAXIMUM_ARRAY_SIZE )
    {
        intNewSize = intMAXIMUM_ARRAY_SIZE;
    }

    //////////////////////////////////////
    //     ---> HUGE BUG HERE <---      //
    //     Code works but is WRONG      //
    // WHY IS THIS HELP ME FIND THE BUG //
    //////////////////////////////////////

    m_intArraySize = intNewSize;
    m_paintValues = new int [m_intArraySize];

    // Initialize to zero
    for( intIndex = 0; intIndex < m_intArraySize; intIndex++ )
    {
        *( m_paintValues + intIndex ) = 0;
    }
}

Is there any particular reason you're using *( m_paintValues + intIndex ) instead of m_paintValues[intIndex]? Was it just to explore array-pointer equivalence? Better yet would be new int [m_intArraySize](), which would zero-initialize it for you. — Shoaib
– Shoaib, Commented Jun 13, 2012 at 3:42
Is there any reason you are introducing bugs when you could be using std::vector? — David Rodríguez - dribeas
– David Rodríguez - dribeas, Commented Jun 13, 2012 at 3:44
I'm not familiar with that method, could you possibly enlighten me? — Neth Munson
– Neth Munson, Commented Jun 13, 2012 at 6:52
please keep in mind this is just a single subroutine in my program I have a CResizableArray.cpp, CResizableArray.h and CHomework8.cpp file This particular subroutine is made ONLY to set the size of the array dynamically. It has been written this way for boundary checking. — Neth Munson
– Neth Munson, Commented Jun 13, 2012 at 7:00

Ernest Friedman-Hill · Accepted Answer · 2012-06-13 03:39:31Z

8

Presumably before this line

m_paintValues = new int [m_intArraySize];

m_paintValues pointed to another array. That array has now been leaked -- you don't have a pointer to it, so it can never be freed. That memory can therefore never be reused. Write a program that does a lot of this, and it'll run out of memory before running very long.

When you're through with a block of memory, you need to free it. Here, the proper thing to do might look something like

delete[] m_paintValues;
m_paintValues = new int [m_intArraySize];

There are more issues, though. First of all, you can never use delete[] unless you know that m_paintValues definitely points to an array; you could ensure that in the constructor. More troubling is that fact that when you set a new size, any data previously in m_paintValues is discarded -- don't you want to copy the old values into the new array? Doing so would mean using a temporary variable to hold the new array when first allocated, copying the data, and then assigning the new array to the member variable.

answered Jun 13, 2012 at 3:39

Ernest Friedman-Hill

81.9k11 gold badges158 silver badges192 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Neth Munson Over a year ago

Wow that's great also you can tell its pointing to an array because of my prefixing methods that is actually intValues as in an integer named Values with a meaning array and p meaning pointer we know it is a pointer array int Values or paintValues

Ernest Friedman-Hill Over a year ago

@NethMunson -- I didn't mean that you needed to be able to tell what type of variable it is; I meant that you can't call delete[] unless you know that the value of the variable is actually a valid pointer to a real array at the time that you make the call. If the pointer is 0 or otherwise points to garbage or to a previously freed array, then calling delete[] will probably crash the program.

AndersK · Accepted Answer · 2012-06-13 05:19:37Z

2

He may mean that since it is a resize you should keep the old contents of the array and transfer them over to the new array, in your snippet you just throw away the old content creating a new empty array.

so instead of

m_paintValues = new int [m_intArraySize];
// Initialize to zero
for( intIndex = 0; intIndex < m_intArraySize; intIndex++ )
{
    *( m_paintValues + intIndex ) = 0;
}

do

int* newBiggerArray = new int[m_intArraySize];
for (intIndex = 0; intIndex < m_intArraySize; ++intSize)
{
   if ( intIndex < oldMaxSize )
   {
     newBiggerArray[intIndex] = m_paintValues[intIndex];
   }
   else
   {
     newBiggerArray[intIndex] = 0;
   }
}
delete [] m_paintValues;
m_paintValues = newBiggerArray;

I will leave the part to handle a resize to a smaller value than previous for you to figure out.

edited Jun 13, 2012 at 5:19

answered Jun 13, 2012 at 3:41

AndersK

36.1k6 gold badges64 silver badges86 bronze badges

1 Comment

Neth Munson Over a year ago

unfortunately no that is not the purpose of this program. My program does exactly what it was built to do, we are just now learning about pointers so he kept it simple.

Collectives™ on Stack Overflow

Resizable Array with pointers

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related