ここから本文です

C++のプログラミングについてです。

アバター

ID非公開さん

2019/6/2601:58:37

C++のプログラミングについてです。

以下のコードは作成途中のものなのです。コメントで「ここの表示がおかしい」と書かれているところは山札と手札を足したものが表示されなければならないのですが何も表示されません。試しにカード枚数を表示させてみると8と表示されるところが−〇〇〇〇〇〇〇〇とデタラメな数字が表示されました。2項演算子内ではちゃんと表示されたのにreturnで返したあとに表示するとおかしくなります。どこが間違えているのか教えてください!

#include<iostream>
using namespace std;

#define MAX_NUM 100 //カードの最大枚数

class Cards {
char *name[MAX_NUM];
int number;
public:
Cards() { number = 0; }
Cards(const Cards &ob);
~Cards(){
for (int i = 0;i < number;i++) {
delete[] name[i];
}
}
void add(const char *str);
void cls();
void show();
Cards operator+(Cards &ob);
};

Cards::Cards(const Cards &ob) {
int l;
for (int i = 0;i < ob.number;i++) {
l = strlen(ob.name[i])+1;
name[i] = new char[l];
if (!name[i]) {
cout << "メモリ割り当てエラー\n";
exit(1);
}
strcpy(name[i], ob.name[i]);
}
}

void Cards::add(const char *str) {
name[number] = new char[strlen(str) + 1];
if (!name[number]) {
cout << "メモリ割り当てエラー\n";
exit(1);
}
strcpy(name[number], str);
number+=1;
}

void Cards::cls() {
number = 0;
}

void Cards::show() {
for (int i = 0;i < number;i++) {
cout << name[i]<<"\n";
}
cout << number;//テスト用
}

Cards Cards::operator+(Cards &ob) {
Cards temp;
for (int i = 0;i < number;i++) {
temp.add(name[i]);
}
for (int i = 0;i < ob.number;i++) {
temp.add(ob.name[i]);
}
//ここでtemp.show()をするとちゃんと表示される。
return temp;
}
int main() {
Cards deck, hand, discard, temp;

deck.add("銅貨");
deck.add("市場");
deck.add("銀貨");
deck.add("礼拝堂");
deck.add("屋敷");
deck.add("金貨");
hand.add("銀貨");
hand.add("地下貯蔵庫");

cout << "山札↓\n";
deck.show();
cout << "\n手札↓\n";
hand.show();

cout << "\ntemp=山札+手札\n";
temp = deck + hand;
cout << "\ntemp↓\n";
temp.show(); //ここでの表示がおかしい
return 0;
}

補足本文の誤字などを修正します。

C++のプログラミングについてです。
以下のコードは作成途中のものです。コメントで「ここでの表示がおかしい」と書かれているところは山札のカードと手札のカードを足したものが表示されなければならないのですが何も表示されません。試しにカード枚数【number】を表示させてみると8と表示されるはずが−〇〇〇〇〇〇〇〇とデタラメな数字が表示されました。2項演算子内ではちゃんと表示されたのにreturnで返したあとに表示するとおかしくなります。どこが間違えているのか教えてください!

閲覧数:
51
回答数:
3
お礼:
100枚

違反報告

ベストアンサーに選ばれた回答

プロフィール画像

カテゴリマスター

n2q********さん

2019/6/2609:54:02

『どこが間違えているのか教えてください!』

了解です。2か所あります。
まず、修正例をお見せしたいと思います。


(1)11行目

【変更前】Cards(const Cards& ob);
【変更後】Cards(const Cards& ob);Cards& operator=(const Cards& ob){this->~Cards();new (this) Cards(ob);return *this;}


(2)25行目

【変更前】for (int i = 0; i < ob.number; i++) {
【変更後】for (int &i = number = 0; i < ob.number; i++) {



~解説~

コピーコンストラクタの実装(2)に誤りがあり、コピー後の this->number が未初期化のまま、値が代入されていません。つまり、this->name の方には文字列がコピーされているのですけれども、その個数である this->number が出鱈目な値になってしまっているというわけです。

あと、代入演算子が実装されていません。(1)に簡易なものを搭載してみました。



《参考意見》


コピーコンストラクタを作成した時は代入演算子も作り込むようにすると良いです。(1)の例は特殊なやり方ですので真似をする必要はありません。ご自身として納得できる分かりやすいやり方で処理すると良いです。

それと、通常は移動用のコンストラクタと代入演算子を実装します。そうすれば効率が良くなります。移動は C++0x(C++11)で可能になりました。たとえば Visual C++ でいうと 2010 から使用可能となっています。

  • アバター

    質問者

    ID非公開さん

    2019/6/2708:52:04

    これのとおりに直したら動きました。ところで(1)の代入演算子はどういう動きをしているのでしょうか?良ければ教えてください

  • その他の返信(5件)を表示

返信を取り消しますが
よろしいですか?

  • 取り消す
  • キャンセル

アバター

質問した人からのコメント

2019/6/28 00:45:22

とても詳しく教えていただきありがとうございました。おかげでエラーがなくなりましたが機能を追加したところまたエラーが発生してしまいました。良ければまた力をお貸しくださいm(__)m

ベストアンサー以外の回答

1〜2件/2件中

並び替え:回答日時の
新しい順
|古い順

prw********さん

2019/6/2613:11:59

>どこが間違えているのか

根本的には「class Cards {」とする思考方法に問題があります

なので、現状はmain関数での宣言で『Cardsとはdeckのことである』と結びついてます

より簡単には、CardクラスとDeckクラスを作って、Deckクラスの中でCardの配列とかvectorとかを使うのがラクです

界隈では、『CardとCardsは全くの別物であり、複数形になると扱いが根本的に異なる』、と考えるの普通です

***

要するに、そのソースコードで「Cards」と書かれているところを全部「Deck」と書き換えても問題が無い、というか、そちらに書き換えた方がより話の通りが良くなってしまう……というのが、最大の問題です

このように書き換えることは出来ますが、それだと「カード」という単語が無くなってしまうので、無意識のうちに忌避している、と思われます

(カードを扱うプログラムなのにカードという単語が無いのはヘンなので)

解決方法は、前述の通りにDeckクラスとCardクラスの二つを作るのが早いです

***

ついでに、CardsすなわちDeckに「たんぽぽサンバ」などの名前を付けたくなった時、やはり(カードの)nameというメンバはCardクラスに属しているべきです

cha********さん

2019/6/2610:50:40

コピー・コンストラクタではなく、アサインメント演算子をオーバーロードする必要があります。前半部分を修正しました。

#include<iostream>
using namespace std;

#define MAX_NUM 100 //カードの最大枚数

class Cards {
char *name[MAX_NUM];
int number;
public:
Cards() { number = 0; }
Cards(const Cards &ob);
Cards& operator= (const Cards &ob); // FIX
~Cards(){
for (int i = 0;i < number;i++) {
if (name[i]) { // FIX
delete name[i]; //FIX
} // FIX
}
}
void add(const char *str);
void cls();
void show();
Cards operator+(Cards &ob);
};

Cards& Cards::operator= (const Cards &ob) { // FIX
number = ob.number; // FIX
int l;
for (int i = 0;i < ob.number;i++) {
l = strlen(ob.name[i])+1;
name[i] = new char[l];
if (!name[i]) {
cout << "メモリ割り当てエラー\n";
exit(1);
}
strcpy(name[i], ob.name[i]);
}
return *this; // FIX
}

この質問につけられたタグ

みんなで作る知恵袋 悩みや疑問、なんでも気軽にきいちゃおう!

Q&Aをキーワードで検索:

Yahoo! JAPANは、回答に記載された内容の信ぴょう性、正確性を保証しておりません。
お客様自身の責任と判断で、ご利用ください。
本文はここまでです このページの先頭へ

「追加する」ボタンを押してください。

閉じる

※知恵コレクションに追加された質問は選択されたID/ニックネームのMy知恵袋で確認できます。

不適切な投稿でないことを報告しました。

閉じる