2016-12-30 6 views
2

私は3人のメンバーで "Student"という名前のクラスを持っています。C++でコピーして削除する

さらに、私はSetNameOperator=の2つの機能を持っています。

class Student 
{ 
private: 
    int ID; 
    char* name; 
    Faculty faculty; 

public : 
bool SetName(const char* other); 

}; 

typedef enum { ENGINEERING, MEDICINE, HUMANITIES, MANAGEMENT, GENERAL } Faculty; 


bool Student::SetName(const char * other) 
{ 

    if (!other) 
     return false; 
    char* temp; 
    temp = new char[strlen(other) + 1]; 
    if (!temp) 
     return false; 
    //if (this->name) 
     // delete[]name; 

    std::string s = other; 
    name = temp; 
    memcpy(name,other,strlen(other) + 1); 


    return true; 
} 

Student& Student::operator=(const Student &other) 
{ 

    this->ID = other.ID; 
    this->faculty = other.faculty; 
    this->SetName(other.name); 
    return *this; 
}; 

もちろん、デフォルトとコピーのデストラクタがあります。 「SetNameメソッド」機能に問題がありますが、私はvalgrindの実行すると、それは私に語った -

==4661== HEAP SUMMARY: 
==4661==  in use at exit: 72,710 bytes in 2 blocks 
==4661== total heap usage: 47 allocs, 45 frees, 74,410 bytes allocated 
==4661== 
==4661== 6 bytes in 1 blocks are definitely lost in loss record 1 of 2 
==4661== at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==4661== by 0x402798: Student::SetName(char const*) (in /home/noam/Desktop/Avoda3/temp/myStudentSet) 
==4661== by 0x40264E: Student::Student(Student const&) (in /home/noam/Desktop/Avoda3/temp/myStudentSet) 
==4661== by 0x4011D6: StudentSet::Add(Student const&) (in /home/noam/Desktop/Avoda3/temp/myStudentSet) 
==4661== by 0x400F1D: StudentSet::StudentSet(Student const*, int) (in /home/noam/Desktop/Avoda3/temp/myStudentSet) 
==4661== by 0x4020B6: main (in /home/noam/Desktop/Avoda3/temp/myStudentSet) 
==4661== 
==4661== LEAK SUMMARY: 
==4661== definitely lost: 6 bytes in 1 blocks 
==4661== indirectly lost: 0 bytes in 0 blocks 
==4661==  possibly lost: 0 bytes in 0 blocks 
==4661== still reachable: 72,704 bytes in 1 blocks 
==4661==   suppressed: 0 bytes in 0 blocks 
==4661== Reachable blocks (those to which a pointer was found) are not shown. 
==4661== To see them, rerun with: --leak-check=full --show-leak-kinds=all 

私は

if (this->name) 
     delete[]name; 

SetName機能にコメントを削除し、追加しようと、私はずっと多くの問題があります。

これを解決する方法はありません。 誰かが私を助けたり、ヒントを与えたりすることはできますか?

+10

'char *'の代わりに 'std :: string'を使うと、標準ライブラリによってメモリが確保されます。 – cxw

+0

'class student'に(' char * name'の代わりに) 'std :: string name'フィールドを宣言する必要があります。 –

+0

@cxw Threはchar *でそれを解決する方法はありませんか?今のところ、私は文字列のchar * instedを使用しています。 –

答えて

1

問題は、作成されたStudentオブジェクトがスコープを外れる(破棄される)と、その名前に割り当てられたメモリが削除されないという問題です。クラスにデストラクタを追加し、コンストラクタでNULLへの名前ポインタを初期化し、デストラクタでテストします。 NULLでない場合は、[]を削除してください。幸運にも、あなたのコードはいくつかのクリーンアップを必要とするかもしれませんが、それは別の話です。今の

3

、私はあなたのC++ツールを使用する方法を学ぶことを好む必要があります使用のchar *カントー

をperefer。最初の提案は、std::stringを使用することが望ましいでしょう。

しかし、自分でメモリを管理したいと思ったら、クラスデストラクタのメモリを削除します。

これは、クラスオブジェクトがどのようなスコープ内でもそのように機能します。しかし、SetName(...)に電話しなかった場合はどうなりますか?名前が指しているゴミは削除できません。

class Student 
    :name(nullptr) 
{ 
    ~Student() { delete[ ] name; } 
    .... 

SetName(...)を使用しないと、未知のものを削除しようとはしません。

0

プレC + 11 C++の混合物を貼りたい場合。コンストラクタ、代入演算子、およびデストラクタがメモリを正しく管理することを確認する必要があります。次のコードは、任意の漏洩なしvalgrid下で実行されます:

#include <string.h> 

typedef enum { ENGINEERING, MEDICINE, HUMANITIES, MANAGEMENT, GENERAL } Faculty; 

class Student 
{ 
private: 
    int ID; 
    char* name; 
    Faculty faculty; 

public : 
    Student() : name(0) {} 
    ~Student() { 
    if (name) 
     delete [] name; 
    } 
    bool SetName(const char* other); 
}; 

bool Student::SetName(const char * other) 
{ 
    if (!other) 
     return false; 

    const int len_other_with_null = strlen(other) + 1; 
    char *temp = new char[len_other_with_null]; 

    if (!temp) { 
     return false; 
    } 

    if (this->name) 
     delete [] name; 

    name = temp; 
    memcpy(name, other, len_other_with_null); 

    return true; 
} 


int main(int argc, char ** argv) { 
    Student student; 
    student.SetName("Hi"); 

    return 0; 
} 
0

あなたが直面していたdelete []名に問題が名前なしで学生を作成の直接の結果です。

Student student; // student.name is uninitialized. Accessing its value 
       // would be undefined behaviour. 

初めてSetNameを使用しようとすると、あなたがuninitialiised値を持っており、それがクラッシュにつながる削除。したがって、あなたはdelete[]を省略し、メモリリークを引き起こします。

おそらく、student.nameは、NULLポインタとしての寿命を開始すると予想しましたが、これはC++が定義されている方法ではありません。あなたは自分でそれをしなければなりません。

イディオム的なC++の方法は、コンストラクタで初期化することです。

class Student { 
    public: 
    Student : name(nullptr) {} 
    ... 

今、あなたはdelete

は、このバグはあなたのための重要な教訓とすることをコメントを解除することができます。常にクラスのコンストラクタを記述し、常にその中のすべてのデータメンバーを初期化します。

これは次の点です。 IDと教員をどのように初期化しますか?デフォルトの「空の」値はありません。たとえあったとしても、身分証明書がなく、教員がいない学生は、何らかの幽霊であることがあります。私たちは対処したくありません。

慣用的なC++の方法は、これらの値をパラメータとしてコンストラクタに渡すことです。

しかし、私たちは無名の生徒を作成しました。これにより、あらゆる種類の問題が発生する可能性があります。おそらくコンストラクタにも名前を渡しますか?

class Student { 
    public: 
    Student (const char* name_, int ID_, Faculty faculty_) : 
     name(nullptr), ID(ID_), faculty(faculty_) { 
     SetName(name_); 
    } 
    ... 

ここは、私たちが誇りに思うことのできる学生です.1つのことを除いて。私たちはまだ名前としてヌルポインタを渡すことができ、生徒は無名のままになります。 SetNameでは、エラーを示すためにfalseを返すことで対処します。コンストラクターは値を返しません。何をすべきか?

熟語C++の方法はで、例外はです。

class Student { 
    public: 
    Student (const char* name_, int ID_, Faculty faculty_) : 
     name(nullptr), ID(ID_), faculty(faculty_) { 
     if (!name_) 
      throw std::domain_error("Null name passed to Student::Student"); 
     SetName(name_); 
    } 
    ... 

あなたのクラスには、3の規則に従ってコピーコンストラクタを持つ必要があります

class Student { 
    public: 
    Student(const Student& other) : 
     Student(other.name, other.ID, other.faculty) {} 
    Student (const char* name_, int ID_, Faculty faculty_) : 
     name(nullptr), ID(ID_), faculty(faculty_) { 
     if (!name_) 
      throw std::domain_error("Null name passed to Student::Student"); 
     SetName(name_); 
    } 

は、実際のコードではもちろん、手動でCスタイルの文字列を管理するのではなく、std::stringを使用します。

関連する問題