c++ delete演算子の使い方

Question

###前提・実現したいこと
下記のコードのようにnew演算子でobjを利用して、delete演算子でobjを消そうとしましたがエラーが発生しています。この場合、どのようにしてobjを消せば良いのでしょうか。

###発生している問題・エラーメッセージ
Main.cpp:10:29: warning: ISO C++11 does not allow conversion from string literal to 'char ' [-Wwritable-strings]
char Worker::CompanyName = "ABC";
^
Main.cpp:21:16: error: use of undeclared identifier 'obj'
delete [] obj;
^
1 warning and 1 error generated.
###ソースコード

<ヘッダー>
class Worker{
 public:
    static int no;
    int num;
    static char* CompanyName;
    double salary;
    char department[100];
    char name[100];
    Worker();
    ~Worker();
    Worker(double salary ,char department[100], char name[100]);
    void ShowData();
    void ShowDataPointer(Worker *a);
    Worker* inputdata();
};
＜メイン文＞
#include <iostream>
using namespace std;

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

int Worker::no = 0;
char* Worker::CompanyName = "ABC";

Worker::Worker(){
 this->num = no;
 this->salary = 0;
 strcpy(this->department, "NotDefinedDepartment");
 strcpy(this->name, "NotDefinedName");
 no++;
}

Worker::~Worker(){
    delete []  obj;
}

Worker::Worker(double salary, char department[100], char name[100]){
  this->num = no;
  this->salary = salary;
  strcpy(this->department,department);
  strcpy(this->name,name);
  no++;
}

 void Worker::ShowData(){
    cout << "no = " << this->num << "\n";
    cout << "salary = " << this->salary << "\n";
    cout << "department = " << this->department << "\n";
    cout << "name = " << this->name << "\n";
 }


void Worker::ShowDataPointer(Worker *a){
    cout << "no = " << a->num << "\n";
    cout << "salary = " << a->salary << "\n";
    cout << "department = " << a->department << "\n";
}

Worker* Worker::inputdata(){
    Worker* obj = new Worker();
    obj->num = no;
    obj->salary=100;
    strcpy(obj->department, "inputdata");
    strcpy(obj->name, "name_inputdata");
    no++;
    cout << "this is inputdata" << "\n";
    obj->ShowData();
    cout << "this is end of inputdata" << "\n";
   // return &obj;  関数を抜けるとき、アドレスが解法されるので下記のように行う必要がある。
                    //new で動的にオブジェクトを作成しないと、ほかの値が作成されると前回のメモリ空間の値が、上書きされる。
    return obj;
}

int main(void){
  Worker w[3];
  Worker w1;
  Worker* w2;
  Worker* someone[3];
  
  for(int i=0 ; i<3 ;i++){
      w[i].salary = 100;
      strcpy(w[i].department, "E");
      strcpy(w[i].name, "Takayuki");
      someone[i] = &w[i];
      cout << "salary = " << someone[i]->salary << "\n";
}
   cout << "---------w2 inputdata;------------" << "\n";
   w2 = w1.inputdata();  //w1は関数を抜けたとき、削除されるか、他のオブジェクトの占有され値が変更されるので、objを返すと良い。
   
   cout << "w1 ===== " << &w1 << "\n";
   cout << "---------w2.ShowDataPointer();------------" << "\n";
   w1.ShowDataPointer(w2);
   cout << "--------- w2->ShowData();------------" << "\n";
   w2->ShowData();
   cout << "---------w1.ShowData();------------" << "\n";
   w1.ShowData();
   cout << "---------w[0].ShowData();------------" << "\n";
   w[0].ShowData();
   cout << "---------------------" << "\n";
   w[1].ShowData();
  cout << "---------------------" << "\n";
  w[0].ShowDataPointer(&w[0]);
  
}

###補足情報(言語/FW/ツール等のバージョンなど)
paiza.io

Answer 1

Worker::~Worker()から有効なスコープ内に、objという変数名が存在しないので、エラーになっています。
inputdata()メソッド内の new 演算子で確保したオブジェクトを、メンバ変数などに保存して、それを delete 演算子に渡して下さい。

Answer 2

こんにちは。

なんか迷走始めてしまいましたね。
現状の構造はオブジェクト指向的にはありえない構造に見えます。inputdata()の役割を明確にしましょう。

このinputdata()は、①Workerインスタンスを生成し、②そこへ値を設定し、③そのポインタを返却していますが、下記疑問があります。

①何故、inputdate()は自分自身(this)のメンバ変数を設定しないのでしょうか？
わざわざWorkerインスンタスを生成して、そこへ設定しているのは何故でしょうか？
②inputdate()の設定対象が自分自身ではダメとしても、呼び出し元で確保したWorkerインスタンスへ値を設定するのではダメですか？
③何故、inputdate()はWorkerインスタンスへのポインタを返却するのでしょうか？
ポインタではなく、Workerインスタンスそのものもを返却するのではダメですか？

メモリの獲得と解放はきちんと設計しないと直ぐに破綻してしまいますので、要件を適切に定義しないとうまくいかないです。

Comments

[saito.kaz]

Worker インスタンスを返却するというのは、単純に
Worker obj;
obj.name = "ABC";
(中略)
return obj;
のようにすればよいのでしょうか？

上記の場合、関数が終了と同時にobjは消えるが、メイン文で下記のように受け取れば受け取れるということでしょうか。

またそもそもメンバ関数にする必要はなく、関数として宣言した方がよろしいですか。

[Chironian]

概ねその通りですが、戻り値もWorkerにします。
Worker Worker::inputdata(){
　　Worker obj; // objがデフォルト・コンストラクタで生成されるのでnoが+1されます。
　　(中略)
　　return obj;
}
なお、Workerのコンストラクタが呼ばれた回数をカウントしているので、Workerの生成回数には注意下さい。return obj;で呼ばれるのはコピー・コンストラクタかムーブ・コンストラクタ（今回のケースではコンパイラが自動生成しますが、当然no++は含まれないです。）の筈なので大丈夫とは思いますが。

受け取りは、下記が好ましいです。
Worker w2=w1.inputdata(); // w2はコピー or ムーブ・コントラクタで生成されます。

下記でも良いですが、w2が余分にコンストラクトされます。
Worker w2; // w2がデフォルト・コンストラクタで生成されるのでnoが+1されます。
w2=w1.inputdata();

> またそもそもメンバ関数にする必要はなく、関数として宣言した方がよろしいですか。

現状の機能ならば、メンバ関数にする必要はないですね。
でも、私なら、特殊な事情がなければinputdate()で自分自身(this)へデータを設定するだろうと思います。その特殊事情の有無が分からないのでなんとも言えません。

[saito.kaz]

ありがとうございます。
参考になります。

Answer 3

Main.cpp:21:16: error: use of undeclared identifier 'obj'
delete [] obj;
^

objがWorkerクラスのメンバーでないためエラーになっています
objが有効なのはinputdata関数の中だけです。
Worker *objをクラスのメンバー変数にすれば
コンパイルエラーは出なくなります。

ただ、Workerクラスのインスタンスを破棄するときに
自分自身でdeleteしようとしているように見えますが
そもそもWorkerクラスのインスタンスの破棄命令を出すのがdeleteなので
Workerクラスのデストラクタでdeleteは無理があります
※C++にJavaのようなガベージコレクタはないので、
参照元がなくなったからと言って勝手にデストラクタが動くことはありません。

あとdelete[]は対象が配列の時に使用するものなので
対象が配列になっていない時はdeleteを使用してください

Comments

[saito.kaz]

ありがとうございます。
勉強になります。