CaliChzHedz
CaliChzHedz

Reputation: 15

Where is my memory leaking in my program and how would I fix it?

so i'm working on an assignment for my c++ class and I am attempting to do different functions using my class with arrays. It works mostly until the overload operator where the memory leak seems to happen.

#include <iostream>
#include <algorithm>  // for std::copy_n
using namespace std;


class MyArray{
private:
double* a{};
int n{};
public:

MyArray(){
    a = nullptr;
    n = 0;
}   // default constructor
/////////////////////////////////////////////////
explicit MyArray(int size){
    n = size;
    a = new double[size]{};

}  //constructor specifying size
/////////////////////////////////////////////////
MyArray(int size, double def_val) {

    n = size;
    a = new double[size];
    for (int i = 0; i < size; ++i) {
         //def_val=i;
        a[i] = def_val;
    }
} //constructor set to default values and specifying size
/////////////////////////////////////////////////
double  GetA(int i){
    return a[i];
}     //setter
/////////////////////////////////////////////////
int GetN() const{
    return n;
}      //getter
/////////////////////////////////////////////////
void  SetN(int x) {
    n = x;
}     //setter
/////////////////////////////////////////////////
void  SetA(int i, double x) {
    a[i] = x;
}     //getter
/////////////////////////////////////////////////
MyArray(const MyArray & a2, int n) {

    a = new double[n]{};
    n = a2.n;
    for (int i=0; i< n; i++){
        a[i]=a2.a[i];
    }
}     //constructor by copy
/////////////////////////////////////////////////
~MyArray(){
    delete []a;
        } // destructor
/////////////////////////////////////////////////
 MyArray operator=(const MyArray& a2) {

        int new_n = a2.n;
        auto *new_data = new double[new_n];
        std::copy_n(a2.a, new_n, new_data);

        delete[] a;

        n = new_n;
        a = new_data;

        return *this;

}  //assignment
int  get_Size() const{
    return n;
}          //accessor
/////////////////////////////////////////////////
void  push_begin(double first) {
    auto* temp = new double [n+1];
    n++;
    for (int i = 0; i < n-1; i++){
        temp[i+1] = a[i];
    }
    temp[0] = first;
    this-> a = temp;
}
/////////////////////////////////////////////////
void  push_end(double last) {
    a[n] = last;
    n++;
} 
/////////////////////////////////////////////////
void  remove_begin() {
    for (int i = 0; i < n - 1; i++) {
        a[i] = a[i + 1];
    }
    n--;
} //done
/////////////////////////////////////////////////
void  remove_End() {
n--;
} 
/////////////////////////////////////////////////
void  reverse() {
    double temp;
    for(int i=0; i<n/2; i++){
        temp = a[i];
        a[i] = a[n-i-1];
        a[n-i-1] = temp;
    }
} 
/////////////////////////////////////////////////
void  display(){
    for (int i = 0; i < n; i++) {
        cout << a[i] << " ";
    }
    cout << endl;
} 

friend MyArray operator + (MyArray &arr1, MyArray &arr2){
    int n1 = arr1.get_Size();
    int m = arr2.get_Size();
    int size = m + n1;
    MyArray ans(arr2, size);
    for(int i=0; i<size; i++){
        ans.push_end(arr2.a[i]);
    }
    return ans;
} 
};

main:

int main(){
MyArray a(9);
MyArray b(10, 5);
b.push_begin(45);
b.push_end(64);
b.push_end(31);
b.push_begin(908);
b.display();
b.display();
b.display();
b.reverse();
b.display();
MyArray c (a+b);
c.display();         //////The leak seems to happen here but I can't find a memory 
                     /////leak finder that works for me
}

I have tried to use valgrind but it doesn't want to run on my laptop.

908 45 8 8 8 8 8 8 64 31
908 45 8 8 8 8 8 8 64 31
908 45 8 8 8 8 8 8 64 31
31 64 8 8 8 8 8 8 45 908
The Max value of the concatenated array is: 908
31 64 8 8 8 8 8 8 45 908 6.93003e-310 6.93003e-310 0 0 0

This is what is output and I have no possible clue where 6.93003e-310 comes from when it should most definitely be 0.

Upvotes: 0

Views: 65

Answers (1)

numzero
numzero

Reputation: 2057

The problem is that your class has no copy constructor. MyArray(const MyArray & a2, int n) is not a copy constructor because it requires an additional argument, n. Which you don’t actually need as you have a2.n.

Also, you leak a in push_begin, and forget to resize it at all in push_end (as well as in setN but you don’t seem to use it at all).

Finally, your operator+ is indeed flawed in several ways. It could be written in one of several ways:

  1. Create an empty new array, then push_back elements from arr1, then push_back elements from arr2.
  2. Create a copy of arr1, then push_back elements from arr2.
  3. Create an array of size arr1.n + arr2.n and fill it directly, without push_back.

What is in the code is a weird mix of these 3 approaches.

And a stylistic note: operator= should return a reference (as pointed out in comments), and operator+ should accept const references.

Still, this is better than most homeworks I’ve seen here on SO.

Upvotes: 1

Related Questions