C++ ; 引数付きコンストラクタ

Question

###前提・実現したいこと
引数付きのコンストラクタへ、ポインタ型のplanを渡したいのですが、
実行結果がplanがうまく表示されません。
どのように記述したら、この問題が解決されるのでしょうか。

###発生している問題・エラーメッセージ
this is constrcuter
----------p2.ShowPhone();-------------------
i = 0
plan =
pInfo = nokia
----------p3.ShowPhone();-------------------
i = 0
plan =
pInfo = nokia

i = 0
plan = normal
pInfo = nokia

This is ShowData

###ソースコード

＜メイン文＞
#include <iostream>
#include <string.h>
#include "Phone.h"

using namespace std;

Phone::Phone(){
    plan = new char[100];
    cout << "this is constrcuter "<< "\n";
    this->i = false;
    strcpy(this->plan, "normal");
    this->pInfo = "nokia";
}

Phone::Phone(bool i, char* plan, string pInfo){
 plan = new char[100];
  this->i = i;
  strcpy(this->plan, plan);
  this->pInfo = "nokia";
}


Phone::~Phone(){
}


void Phone::ShowPhone(){
    cout << " i = " << this->i << "\n";
    cout << " plan = " << this->plan <<"\n";
    cout << " pInfo = " << this->pInfo <<"\n";
}

void Phone::ShowData(){
    cout << "  This is ShowData  " << "\n";
}


int main(){
    Phone p1;
    Phone p2(false, "p2","noraml2");
    Phone p3(false, "p3","noraml3");
    cout << "----------p2.ShowPhone();-------------------" << "\n";
    p2.ShowPhone();
    cout << "----------p3.ShowPhone();-------------------" << "\n";
    p3.ShowPhone();
    cout << "-----------------------------" << "\n";
    
    
    p1.i = false;
    strcpy(p1.plan,"normal");
    p1.pInfo = "nokia";
    p1.ShowPhone();
    cout << "-----------------------------" << "\n";
    p2.ShowData();
    
    return 0;
}

＜ヘッダー文＞
#include <string.h>

class Phone{
  public: 
    Phone();
    Phone(bool i, char* plan, std::string pInfo);
    ~Phone();
    bool i;
    char* plan;
    std::string pInfo;
    void ShowPhone();
    virtual void ShowData();
};

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

Answer 1

Phone::Phone(bool i, char* plan, string pInfo){
plan = new char[100]; /ここでplanの中身が消されてます/
this->i = i;
strcpy(this->plan, plan);
this->pInfo = "nokia";
}

Comments

[saito.kaz]

この場合、どのように変更すれば良いのでしょうか。
例えば、下記のコメントに記したように書いたのですが、うまく行きません。
実行結果が同じとなりました。

すいませんが教えてください。

Answer 2

こんにちは。

バッとみただけですが、下記はNGです。

Phone::Phone(bool i, char* plan, string pInfo){
plan = new char[100];

このplan = new char[100];はパラメータとして渡されたplanを書き換えてしまっています。
this->plan = new char[100];が正解ではないでしょうか？

このようなミスが発生しやすいのでthis->を使うのではなく、メンバ変数名と引数名を異なるような命名規則で運用することをお勧めします。

例えば、私は可能な時は、メンバ変数の頭にm、入力引数の頭にiをつけるようにしています。
この場合、iPlanとmPlanってつけてます。参考までに。

Comments

[saito.kaz]

いつもありがとうございます。本当に勉強になります。
コンストラクタを下記のように変更しましたが、結果が同じです。

どのように、別変数を記入すれば良いのでしょうか。

Phone::Phone(bool i, char* mplan, string pInfo){
　mplan = new char[100];
this->i = i;
strcpy(this->plan, mplan);
this->pInfo = "nokia";
}

[saito.kaz]

Phone::Phone(bool i, char* plan, string pInfo){
　this->plan = new char[100];
this->i = i;
strcpy(this->plan, plan);
this->pInfo = "nokia";
}

試したところ、うまく行きました。
ありがとうございます。

ですがこのやり方がなぜいけないのですか。
コンストラクタなのでthis->ポインタを使うことで不具合が起きないと思うのですが、どうでしょうか。

初心者なので質問が多くて申し訳ありません。

[Chironian]

> Phone::Phone(bool i, char* mplan, string pInfo){
>　mplan = new char[100];

main()から渡された「"p2"や"p3"の先頭アドレス」がmplanに設定されているのですが、mplan=new char[100];でそれを消してしまってますよ。折角受け取った値を上書きしてはダメと思いませんか？（また、この場合mplanはメンバ変数ではないですよ。Javaでも同じでしょう？）
他の部分を見る限りTAKAYUKI_MIWAさんはこの辺りについては理解されているようですので、ケアレスミスと思うのですが．．．

[HiroshiWatanabe]

＞コンストラクタを下記のように変更しましたが、結果が同じです。
＞どのように、別変数を記入すれば良いのでしょうか。
＞Phone::Phone(bool i, char* mplan, string pInfo){
＞　mplan = new char[100];

そうではなく、this->を付けずにするなら

＞Phone::Phone(bool i, char* mplan, string pInfo){
＞　plan = new char[100];

ということです。
planはクラスのメンバなので引数で受け取るのはplanではなくmplanに変更した
(this->をつけなくてもメンバ変数か引数かが明確に区別できる状態にする）
という対応策ですね。
※plan=new char[100];はクラスのメンバ変数を初期化したいというニーズだからこちらのplanまでmplanにしてしまうと引数のmplanを指定する事になってしまうので改名前と何も変わらず問題が解決しません