0

I'm trying to make a copy constructor with low level arrays and I'm getting a core dumped error when using delete, can't find out a solution because I'm not able to use std::vector to make this. Can you guys help me ?? =)

#include<iostream>
#include<string>
#include<initializer_list>
class Vector{
    size_t n;
    double* datos;
public:
    Vector(size_t n_,double);
    Vector(const std::initializer_list<double>&l);
    void show();
    ~Vector(){
        delete[] datos;
    }
    Vector(Vector&& other):n(other.n){
        delete [] datos;
        datos =other.datos;
        other.datos =nullptr;
    }
    Vector(const Vector& v);
    Vector operator = (const Vector& v);
};
/* --------------------------- METODOS DE LA CLASE -------------------------- */
Vector::Vector(const Vector&v){
    delete[]datos; //CORE DUMPED
    n=v.n;
    datos = new double[n];
    
    for (size_t i = 0; i < n; i++)
    {
        datos[i] = v.datos[i];
    }
    std::cout<<std::endl;
    
}
Vector Vector::operator = (const Vector& v){
    delete [] datos;//CORE DUMPED
    n = v.n;
    for (size_t i = 0; i < n; i++)
    {
        datos[i] = v.datos[i];
    }
    return *this;
}
Vector::Vector(const std::initializer_list<double>&l):n(l.size()),datos(new double[n]){
    size_t j= 0;
    for (auto i:l)
    {
        datos[j]=i;
        ++j;
    }
}

void Vector::show(){
    for (size_t i = 0; i < n; i++)
    {
        std::cout<<datos[i]<<", ";
    }
    std::cout<<std::endl;
}
Vector::Vector(size_t n_,double d=0):n(n_),datos(new double[n]){
    if (n < 1)
    {
        throw std::invalid_argument("Wrong size!");
    }
    
    for (size_t i = 0; i < n; i++)
    {
        datos[i] = d;
    }
}
int main(){
    Vector b={2,3,4,5,6},a(3);
    a=b;
    a.show();
}

Using POP OS 21.04 (just in case this can help).

Please don't be rough with me I'm a junior programmer trying to pass September's exams =(

  • 3
    `delete[]datos; //CORE DUMPED` -- Ask yourself -- Why are you calling `delete[]` here in the copy constructor? Do you know what the purpose of a copy **constructor** is? If it is to construct a new object, what is there to `delete[]`? The pointer is uninitialized anyway, but logically, why did you believe you need to delete anything there? – PaulMcKenzie Aug 27 '21 at 10:13

2 Answers2

2
Vector::Vector(const Vector&v){
    delete[]datos; //CORE DUMPED

You didn't initialise datos, so its value is indeterminate. When you delete an indeterminate pointer, then the behaviour of the program is undefined. "CORE DUMPED" is one possible behaviour that you may observe.

eerorika
  • 232,697
  • 12
  • 197
  • 326
1

You are issuing unneeded calls to delete [] datos in your constructors (move and copy).

Since datos is uninitialized, calling delete [] on an uninitialized pointer leads to undefined behavior -- in your case, your program crashes.

Since the objects are being constructed, there is no reason to issue a delete [] on the pointer, since the object this is brand new.

Simply remove the call to delete [] datos; from the constructors. Whether this is the only problem is another story, but it is an existing one.


In addition, your assignment operator:

Vector Vector::operator = (const Vector& v)

is also incorrect. It fails to allocate new memory after the delete [] call, thus the for loop that is written will write into unallocated memory. Also, it should return a reference to the current Vector object, not a brand new Vector object.

The easiest way to implement the assignment operator is to use std::swap:

#include <algorithm>
//...
Vector& Vector::operator = (Vector v)
{
    std::swap(v.datos, datos);
    std::swap(v.n, n);
    return *this;
}

This assumes you have a working, non-buggy copy constructor and destructor for Vector. See the copy / swap idiom for details on why this works.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45