views:

153

answers:

5

I wrote my first ever C++ template code on expandable array and I am getting a segmentation fault! After an hour of debugging I have realized that I need help. Something is wrong with the constructor or the destructor I think but not sure.

The code is on pastie ready to be compiled. http://pastie.org/1150617

/* Expandable array in C++ */

#include <iostream>
using namespace std;

template <class T>
class EArray{
private:
    T* arr;
    int size;
public:
    EArray(int l);
    ~EArray();

    void setElement(int i, const T& newval);
    void eraseElement(int i);
    void addElement(int i, const T& newval);
    void push(const T& newval);
    void display();
};

template <class T>
EArray<T>::EArray(int l){
    size = l;
}

template <class T>
EArray<T>::~EArray(){
    delete [] arr;
    arr = NULL;
}

template <class T>
void EArray<T>::setElement(int i, const T& newval){
    if(i < size && i >= 0){
        arr[i] = newval;
    }
}

template <class T>
void EArray<T>::eraseElement(int index){
    size -= 1;
    T* newarr = new T[size];
    for (int i = 0; i < size+1; i++){
        if (i < index){
            newarr[i] = arr[i];
        }
        else if(i > index){
            newarr[i-1] = arr[i];
        }
    }
    delete [] arr;
    arr = newarr;
}

template <class T>
void EArray<T>::addElement(int index, const T& newval){
    size += 1;
    T* newarr = new T[size];
    for(int i = 0; i < size; i++){
        if(i<index){
            newarr[i] = arr[i];
        }
        else if (i == index){
            newarr[i] = newval;
        }
        else{
            newarr[i] = arr[i-1];
        }
    }
    delete [] arr;
    arr = newarr;
}

template <class T>

void EArray<T>::push(const T& newval){
    size += 1;
    T * newarr = new T[size];
    for (int i = 0; i < size-1; i++){
        newarr[i] = arr[i];
    }
    newarr[size-1]=newval;
    delete [] arr;
    arr = newarr;
}

template <class T>
void EArray<T>::display(){
    for(int i = 0; i < size; i++){
        cout << arr[i] << endl;
    }
}

int main(){
    EArray<int> A(6);
    A.setElement(0,34);
    A.setElement(1,544);
    A.setElement(2,32);
    A.setElement(3,324);
    A.setElement(4,24);
    A.display();
    A.addElement(3,12);
    A.display();
    A.eraseElement(4);
    A.display();
    A.push(32456);
    A.display();
}
+11  A: 

It has nothing to do with templates. It's just a problem of memory management. In the constructor of EArray, you have never initialized arr, so by default it contains some invalid pointer.

But then in setElement, you used this invalid pointer arr[i] = newval;, which should cause a SegFault.

It should be fixable by adding

arr = new T[size];

in the constructor (result: before, with segfaultafter, running fine).

(BTW, in practice, please use a std::vector.)

KennyTM
yes, I wrote this just to understand the mechanics. I would use st::vector for usage. Thanks for the answer.
zubinmehta
A: 

Your ctor does NOT allocate memory for the elements. And you are using setElement before pushing elements into the object. So, arr[i] in setElement will access unallocated memory, and AV.

Benedetto
+1  A: 

Your EArray constructor does not initialize arr

add arr = new T[size]; after line 24

Or change it to:

template <class T>
EArray<T>::EArray(int l) : size(l), arr(new T[size]){
    size = l;
}

You should provide a correct copy construtor and assignment operator as well - or make them private as to not allow your EArray to be copied around.

nos
A: 

Well 1 thing to remember is that you specify there is a return of an int in your main function then don't return anything. This is wrong.

Your error, however, arises from the fact that you set the size of the array in the constructor but don't actually allocate the space for it. So when you try to set the first element it tries to set it to unallocated memory and thus you end up with a segfault.

Goz
Your first point is incorrect. `main` is allowed to have an implied `return 0;` tacked on at the end if you don't return anything.
Dennis Zickefoose
+1  A: 

I'm still at it. But a first error is:

  • the constructor does not allocate, but only sets size
  • setElement does access the field while it is not allocated.

(that seems to be it)

Ronny