こんにちはゲストさん。会員登録(無料)して質問・回答してみよう!

解決済みの質問

デストラクタについて

#include <iostream>
#include <string>

using namespace std;

#define NUM 2 //登録人数

class Person{
char *name;
int *age;
char *hobby;
public:
Person()
{
name = new char;
age = new int;
hobby= new char;
}
void Set(char *n,int a,char *h)
{
name=n;
*age=a;
hobby=h;
}
char *Get_name(void)
{
return name;
}
int Get_age(void)
{
return *age;
}
char *Get_hobby(void)
{
return hobby;
}
~Person()
{
cout<<name<<"のデストラクタ\n";
delete [] name;
delete age;
delete [] hobby;
}
};
int main(void)
{
Person *p;
int i;

p=new Person[NUM];

p[0].Set("永嶋",21,"映画鑑賞");
p[1].Set("平林",54,"車");

for(i=0;i<NUM;i++){
cout<<"\n名前:"<<p[i].Get_name();
cout<<"\n年齢:"<<p[i].Get_age();
cout<<"\n趣味:"<<p[i].Get_hobby()<<"\n";
}

delete [] p;

return 0;
}
というプログラムを作成したのですが
デストラクタの3つのdeleteがおかしいようなのですが
どのような部分が問題となっているのでしょうか?
回答・アドバイス宜しくお願いいたします。

投稿日時 - 2004-11-07 01:36:40

QNo.1073277

困ってます

質問者が選んだベストアンサー

No2補足の回答です。

>Setのメソッドの中にある二つのif文は何のために有るのでしょうか?
>
>初心者が言うのは生意気ですが
>デストラクタの部分でdeleteすれば良いのではないのでしょうか?
同じオブジェクトに対して複数回Setが呼び出されたときに、
以前に確保したname,hobby領域を開放して、新しいname,hobbyの領域を確保しています。
これを行わずいきなりname=new char[strlen(n)+1];とやると、
nameが上書きされるため古いnameが開放できなくなります。

同じオブジェクトに対してSetは絶対に1回しか呼ばないならばこの処理は不要ですが、
そういった条件を設けることはあまり望ましくないと思います。

投稿日時 - 2004-11-07 15:37:10

お礼

やっと意味が分りました。

再度回答ありがとうございました。

投稿日時 - 2004-11-08 03:50:21

ANo.5

このQ&Aは役に立ちましたか?

2人が「このQ&Aが役に立った」と投票しています

回答(5)

ANo.4

眠い頭で回答してしまったのでちょっと日本語と用語の使い方がおかしいですね。申し訳ありませんでした。
もしコンストラクタのnewが変数の型の初期値で初期化したいなら、
age = int();
のようにするか、初期化リストを使うと良いですよ。
Person()
:name (NULL) , age (0) , hobby (NULL)
{

}
それとも、 *Get_hobby()とかをSet()するまえに呼び出したときの保険なら、
Get()の方で判定して
return "";
としてやるのもいいかも。ただし、const char*ですね、コレは。
そうでなければ、毎回きちんとSet()が呼び出されるたびにnew/deleteして
メモリ管理しなければなりません。(他の方の仰っているstringもそれを自動で
やってくれるものです)

投稿日時 - 2004-11-07 10:43:21

お礼

アドバイス有難うございます。

いろいろ勉強できてためになります。

投稿日時 - 2004-11-08 03:49:16

ANo.3

まずコンストラクタの

Person()
{
name = new char;
age = new int;
hobby= new char;
}

でヒープ領域にメモリ確保している意図がわかりません。

この処理ロジックだと、別にヒープにメモリ確保する必要なしかと。


-------------------------------------


#include <iostream>
#include <string>

using namespace std;

#define NUM 2 //登録人数

class Person{

char *name;
int age;
char *hobby;

public:

Person()
{
}

void Set(char *n,int a,char *h)
{
name=n;
age=a;
hobby=h;
}
char *Get_name(void)
{
return name;
}

int Get_age(void)
{
return age;
}

char *Get_hobby(void)
{
return hobby;
}

~Person()
{
cout<<name<<"のデストラクタ\n";
}

};

int main(void)
{

Person *p;
int i;

p=new Person[NUM];

p[0].Set("永嶋",21,"映画鑑賞");
p[1].Set("平林",54,"車");

for(i=0;i<NUM;i++){
cout<<"\n名前:"<<p[i].Get_name();
cout<<"\n年齢:"<<p[i].Get_age();
cout<<"\n趣味:"<<p[i].Get_hobby()<<"\n";
}

delete [] p;

return 0;

}


-------------------------------------

といった感じですかね。

よくわからないうちにむやみに多用するとリークおこしまくりますよ^^;

投稿日時 - 2004-11-07 10:20:40

お礼

自分でも必要ないとは思っていたのですが
正確には私が書いたように白とは書いてないのですが
メソッドの中でnew演算子を使う事が
条件だったのであのようになってしまいました。
今後気をつけます。

回答有難うございました。

投稿日時 - 2004-11-08 03:47:46

ANo.2

char*型の代入はポインタ値がコピーされるだけで、その指す先がコピーされるわけではありません。
関数呼び出しでも文字列そのものが渡されるわけではなく先頭アドレスが渡されます。

上の例だと
p[0].Set("永嶋",21,"映画鑑賞");
としている部分では、メモリ上のどこかに永嶋、映画鑑賞というデータが書き込まれた領域が作成されていて、
その先頭アドレスが例えば100,200だとすると、関数呼び出し部分は
p[0].Set(100,21,200);
と呼び出されることになります。
呼び出されるSet内のname=n;はname=100;ということで、
nameに入っていたコンストラクタでnewした領域のアドレスは失われます。
そしてデストラクタでdeleteする領域がnewで確保した領域でないためエラーとなります。
また、name = new char;は1文字分しか領域を確保していないという間違いもあります。

解決案はstring型を使うか、以下のようにSet時に領域を確保する方法があります。
class Person{
char *name;
int age;//ポインタにする意味がないと思う
char *hobby;
public:
Person()
{
name = NULL;
hobby= NULL;
}
void Set(char *n,int a,char *h)
{
if (name) delete[] name;
name=new char[strlen(n)+1];
strcpy(name, n);
age=a;
if (hobby) {delete[] hobby;}
hobby=new char[strlen(h)+1];
strcpy(hobby, h);
}

投稿日時 - 2004-11-07 02:30:25

お礼

回答有難うございました。

Setのメソッドの中にある二つのif文は何のために有るのでしょうか?

初心者が言うのは生意気ですが
デストラクタの部分でdeleteすれば良いのではないのでしょうか?

見当はずれな事を聞いているかもしれませんが
良い機会なので教えていただければ幸いです。
宜しくお願いします。

投稿日時 - 2004-11-07 10:11:32

ANo.1

いろいろまずいと思います。
まず、コンストラクタで
new char;
などとしているのに、デストラクタで
delete [] name;
などとなっています。
これはまずいです。
それと、
Set()でnに指定されるのはnewされた文字列へのポインタではなく
ヒープのアドレスです。
new / deleteとnew [] / delete [] は対応させないと。

投稿日時 - 2004-11-07 01:50:31

お礼

回答有難うございます。

投稿日時 - 2004-11-07 09:55:42