C++のデストラクタで不明瞭なものがたくさんあります

Question

私は非常に単純なコードをC++で書いて、ベクトルの簡単な操作を行っています。これは、ファイルvector.hの内容です：

#ifndef VECTOR_H_INCLUDED 
#define VECTOR_H_INCLUDED 

class Vector { 
    int *coordinates; 
    int *size; 
public: 
    Vector(int vector_size); 
    Vector(int*,int); 
    ~Vector(); 
    void print(void); 
    Vector operator +(Vector); 
}; 
#endif 

、これは実装です（ファイル：vector.cpp）：私のmain.cppにから

#include "vector.h" 
#include <iostream> 
using namespace std; 

Vector::Vector(int vector_size) { 
    coordinates = new int[vector_size]; 
    size = new int; 
    *size = vector_size; 
} 

Vector::Vector(int* vector_coordinates, int vector_size){ 
    coordinates = vector_coordinates; 
    size = new int; 
    *size = vector_size; 
} 

void Vector::print(void){ 
    cout << "["; 
    for (unsigned short int index =0; index<*size; index++){ 
     cout << coordinates[index]; 
     if (index < *size-1){cout << ", ";}; 
    } 
    cout << "]\n"; 
} 

Vector Vector::operator+ (Vector other) { 
    Vector temp(*(other.size)); 
    if ((*temp.size)!=(*(this->size))){ 
     throw 100; 
    } 
    int* temp_c = new int[*(other.size)]; 
    int* other_c = other.coordinates; 
    for (unsigned short int index =0; index<*size; index++){ 
    temp_c[index] = coordinates[index] + other_c[index]; 
    } 
    temp.coordinates = temp_c; 
    return (temp); 
} 

Vector::~Vector(){ 
    delete[] coordinates; 
    delete size; 
} 

、私は次の操作を行います

#include <iostream> 
using namespace std; 
#include "vector/vector.h" 

const int size = 3; 

int main() { 
    int *xxx = new int[size]; 
    xxx[0]=4; xxx[1]=5; xxx[2]=-6; 


    Vector v(xxx,size);// v = [4, 5, -6] 
    Vector w(size);// w is a vector of size 3 

    w = v+v; // w should be w=[8,10,-12] 
    w.print(); 
    return 0; 
} 

結果がある：

[148836464、5、-6、17、148836384、0、0、17、0、0、0、17、3、0、0、17、0 、0,0,17,1488セグメンテーションフォールト

デストラクタから2行を削除した場合：

delete[] coordinates; 
delete size; 

すべてが期待どおりに動作し、プログラムの出力：

[8, 10, -12] 

私はどんな説明をいただければ幸いです...

アップデート1：私は目に私のオペレータ+方法を変更e。次が、問題が解決されなかった。

Vector Vector::operator+(Vector other) { 
    int size_of_other = *(other.size); 
    int size_of_me = *(this->size); 
    if (size_of_other != size_of_me) { 
     throw 100; 
    } 
    int* temp_c = new int[size_of_me]; 
    int* other_c = other.coordinates; 
    for (unsigned short int index = 0; index < size_of_me; index++) { 
     temp_c[index] = coordinates[index] + other_c[index]; 
    } 
    Vector temp(temp_c,size_of_me); 
    return (temp); 
} 

アップデート2：

Vector Vector::operator+(Vector other); 

私は望ましい結果を得ないだろう：私は演算子を使用していることに気づきました。それは仕事をした修正された：

const Vector& Vector::operator+(const Vector& other) { 
    Vector temp(other.size); 
    for (unsigned short int index = 0; index < size; index++) { 
     cout << "("<< index <<") "<<coordinates[index] << "+" 
         <<other.coordinates[index] << ", "<< endl; 
     temp.coordinates[index] = coordinates[index] + other.coordinates[index]; 
    } 
    return (temp); 
} 

アップデート3：更新＃2の後、私は地元の「TEMP」を返し、コンパイラから警告を得ていました。私は（私は一時のコピーを返す）完全にすべての問題を解決し、以下に私のコードを変更し、正常に動作します：

int* temp_c = new int; 
... 
    temp_c[index] = 

あなたは、次のとおりです。

const Vector Vector::operator+(const Vector& other) const{ 
    Vector temp(other.size); 
    for (unsigned short int index = 0; index < size; index++) { 
     temp.coordinates[index] = coordinates[index] + other.coordinates[index]; 
    } 
    return *(new Vector(temp)); 
} 

Answer 1

あなたVector::operator+は、少なくとも一つのバグがありますインデックスがtemp_cの場合は、1つの整数だけが割り当てられていました。だから、あなたのループが他のメモリに詰まっているため、未定義の動作が起こります。

また、Vectorオブジェクトを正しく使用できるように、コピーコンストラクタを定義する必要があります。コンパイラはデフォルトのコピーコンストラクタを生成しますが、デフォルトのコンストラクタは一般的にポインタを含むオブジェクトには適していません。

この行：それは以前に割り当てられたtemp.coordinatesベクトルを上書きするため

temp.coordinates = temp_c; 

は、メモリリークを引き起こします。

アップデート3：あなたのコード

return *(new Vector(temp)); 

それが動作するように見えるが、まだメモリリークです。新しいVectorを割り当てている場合、コンパイラはコピーコンストラクタを呼び出して、それを関数の戻り値にコピーします。誰も今までdeleteのVectorオブジェクトを持っていないので、メモリリークがあります。

解決策は、コンパイラ生成のデフォルトコピーコンストラクタに頼るのではなく、コピーコンストラクタを書くことです。解決策はです。あなたの質問に対する他の答えはすべて同じことを言っています。正しいプログラムのためにこれを行う必要があります。あなたは割り当て、その後、デストラクタでそれを削除しようとしていなかったデータへのポインタを座標割り当てる

Vector::Vector(int* vector_coordinates, int vector_size){ 
    coordinates = vector_coordinates; 
    size = new int; 
    *size = vector_size; 
} 

：それをやって

Answer 2

は悪い考えです。

しかし、segfaultを取得する本当の理由は、デフォルトのコピーコンストラクタを使用し、vの一時コピーが終了するとベクトルを削除することです。コピーコンストラクターを実装し、深いコピーまたは参照カウントを確実にする必要があります。

このような何か試してください：あなたは、引数としてconst参照を取る場合にも、あなたのオペレータ+は、はるかに効率的である

Vector::Vector(const Vector& other){ 
    size = new int(*other.size); 
    coordinates = new int[size]; 
    memcpy(coordinates, other.coordinates, sizeof(int)*(*size)); 
} 

を：

Vector Vector::operator+ (const Vector& other) 

Answer 3

あなたのクラスはコピーコンストラクタを必要と割り当てをコピーしますオペレーターが正しく動作するようにします。彼らが必要とする大きなヒントは、デストラクタが{}ではないということです。 「Rule of Three」を参照してください。

移動体のコンストラクタと移動代入演算子については、少し上手く現代的になるように考えることもできます。

Answer 4

は、一時オブジェクトは、その後、Wおよび破壊に割り当てられたV + Vの結果、のために構成されているライン

w = v+v; // w should be w=[8,10,-12] 

を考えます。 割り当てオペレータがないため、デフォルトの実装ではシャローコピーが実行され、割り当て解除されたメモリで作業しています。

この問題を解決する簡単な方法は、メンバのメモリを割り当てるときにコピーコンストラクタ/代入演算子とデストラクタを実装することです。

Answer 5

は、以下のコードを試してみてください。

1. は、デフォルトコンストラクタを実装します。これは、あなたのオブジェクトが構築されているにもかかわらず、内部変数がヒープ上の何かを指しているか、NULLにあるので、delete []コールはひどく死ぬことはありません。
2. コピーコンストラクタを実装します。デフォルトのコピーコンストラクタはヒープ上のメモリをコピーしないので、あなたにとって重大な問題になるでしょう。
3. 代入演算子を実装します。この場合も、浅いコピーは避けられます。
4. サイズをポインタとして削除します。ほとんどのシステムでは、ポインタは整数と同じサイズですので、サイズをポインタにするだけで不必要に複雑になります。
5. 中間の割り当てを避けることによって加算コンストラクタを修正します。一時的なローカル変数があるので、いくつかの余分な中間オブジェクトを割り当てる代わりに、それを使用します。

...見て取る：

// VectorImplementation.cpp : Defines the entry point for the console application. 
// 

#include <iostream> 
using namespace std; 


class Vector { 
    int *coordinates; 
    int size; 

public: 
    Vector(); 
    Vector(int vector_size); 
    Vector(int*,int); 
    Vector(const Vector& v); 

    ~Vector(); 

    Vector operator +(Vector); 
    Vector& operator =(const Vector & other); 

    void print(void); 
}; 

Vector::Vector() { 
    coordinates = NULL; 
    size = NULL; 
} 

Vector::Vector(int vector_size) { 
    coordinates = new int[vector_size]; 
    size = vector_size; 
} 

Vector::Vector(int* vector_coordinates, int vector_size){ 
    coordinates = vector_coordinates; 
    size = vector_size; 
} 

Vector::Vector(const Vector& v) { 
    size = v.size; 
    coordinates = new int[size]; 
    memcpy(coordinates,v.coordinates, sizeof(int)*size); 
} 

void Vector::print(void){ 
    cout << "["; 
    for (unsigned short int index =0; index<size; index++){ 
     cout << coordinates[index]; 
     if (index < size-1){cout << ", ";}; 
    } 
    cout << "]\n"; 
} 

Vector Vector::operator+ (Vector other) { 
    Vector temp(other.size); 
    for (unsigned short int index =0; index<size; index++){ 
     temp.coordinates[index] = coordinates[index] + other.coordinates[index]; 
    } 
    return (temp); 
} 

Vector & Vector::operator= (const Vector & other) 
{ 
    if (this != &other) // protect against invalid self-assignment 
    { 
    // 1: allocate new memory and copy the elements 
    int * tmp_coordinates = new int[other.size]; 
    memcpy(tmp_coordinates, other.coordinates, sizeof(int)*other.size); 

    // 2: deallocate old memory 
    delete [] coordinates; 

    // 3: assign the new memory to the object 
    coordinates = tmp_coordinates; 
    size = other.size; 
    } 
    // by convention, always return *this 
    return *this; 
} 

Vector::~Vector(){ 
    printf("Destructing %p\n", this); 
    delete[] coordinates; 
} 

const int size = 3; 

int _tmain(int argc, _TCHAR* argv[]) 
{ 

    int *xxx = new int[size]; 
    xxx[0]=4; 
    xxx[1]=5; 
    xxx[2]=-6; 

    Vector v(xxx,size);// v = [4, 5, -6] 
    Vector w(size);// w is a vector of size 3 

    w = v+v; // w should be w=[8,10,-12] 
    w.print(); 

    return 0; 
}