c++における銀行口座

Question

コンストラクタの中にあるelse文の中で
このようなコードを使い、顧客番号を設定しようとしました。
しかしエラーが出てしまって、うまく機能していません。
文法上どのような誤りがあるのでしょうか？また先輩方にコード全体の改善点を指摘していだけると嬉しいです。
よろしくお願いします
ご指摘ありがとうございました。
コンストラクタが形成されると同時に０－１－２（顧客Aに０、顧客Bに１、顧客Cに２）というように番号を振っていきたかったので、下のコードを書きました。
今見てみると、このコードが機能しないのは分かります。
一つ浮かんだ考えとしてはコンストラクタの引数に顧客番号をいれてしまえばいいのではと思ったのですが、コンストラクタには初期値だけいれる単純なもののほうが好ましいと感じました。
そこで何かほかの方法がありましたら、教えていただけますか？

balance =balanceA ;
int k=0;
Data_number[k]=k++;
}

コード#include<iostream>
using namespace std;
class Bankaccount {
private:
	int balance;
	int Data_number[100];
public:
	Bankaccount(int a);
	int getBalance();
	void setBalance(int);
	int getData_number(int);
	void credit(int);
	void debit(int);

};
int Bankaccount::getBalance() {
	return balance;
}
void Bankaccount::setBalance(int a) {
	 balance = a;
}
int Bankaccount::getData_number(int number){
	return Data_number[number];
}
Bankaccount::Bankaccount(int balanceA) {
	if (balanceA<= 0) {
		balanceA = 0;
		cout << "error";
	} else {
		balance =balanceA ;
		int k=0;
		Data_number[k]=k++;
	}
}
void Bankaccount::credit(int money){
	balance+=money;
}
void  Bankaccount::debit(int money){
	balance-=money;
}
int main() {
	Bankaccount a(3000);
	a.setBalance(4000);
	cout << a.getBalance()<<endl;
	a.debit(300);
	cout<<a.getBalance()<<endl;
	a.credit(1000);
	cout<<a.getBalance()<<endl;
	cout<<getData_number(0);
}

Answer 1

取り敢えず下記を修正すればコンバイルと実行できました。

誤> cout<<getData_number(0);
正> cout<<a.getData_number(0);

単なるタイプミスではないでしょうか？

以下、ざっと私としては気になった点を書いてみます。他の方のご意見も参考にされて下さいね。

Data_numberという配列はなんの配列でしょうか？
もし顧客番号であれば、Customer_number等、意味が分かる名前にした方がよいと思います。
また、これは何故配列なのでしょうか？
そして、配列であれば添字オーバーチェックはやるべきです。getData_number()関数でnumberパラメータ異常の検出です。

下記はやりたいことがよく見えませんでした。
int k=0;
Data_number[k]=k++;
もし、0クリアならば、例えば下記でできます。
for (int k=0; k < 100; ++k)
Data_number[k]=0;

頑張って下さい。ではでは

Comments

[reotantan]

設定としましては、顧客番号は多くなることを予想しまして、配列のほうが効率が良いかなと思いました。添え字オーバーチェックですね、とても反省しています。
回答者様なら顧客番号はどのように設定しますか？

[Chironian]

まず、顧客１人１人毎にbalance(残高?)が必要になると理解しました。
であれば、一般的には顧客番号はBankaccount内に1つだけですので、Data_numberも1つだけとした方が良さそうに思いました。(特殊なケースで１口座に複数の顧客がいるのでしたらごめんなさい。)

次に顧客番号をBankaccountクラス内部で管理するのであれば、静的メンバ変数を使えば良いです。
静的メンバ変数について解説しているサイトは多いので参考にされて下さい。
例えば→http://ppp-lab.sakura.ne.jp/cpp/language/019.html

以下、修正点です。
reotantanさんがローカル変数kを静的メンバ変数へ変えただけです。ただ、メンバ変数名として"k"はあんまりですので、適切な名称に変更するべきです。

#include<iostream>
using namespace std;
class Bankaccount {
private:
int balance;
int Data_number;
static int k;
(中略)
};
(中略)
int Bankaccount::getData_number(){
return Data_number;
}
int Bankaccount::k=0;
Bankaccount::Bankaccount(int balanceA) {
if (balanceA<= 0) {
balanceA = 0;
cout << "error";
} else {
balance =balanceA ;
Data_number=k++;
}
}
(中略)
int main() {
Bankaccount a(3000);
(中略)
cout<<a.getData_number()<<endl;

Bankaccount b(2000);
cout<<b.getData_number()<<endl;
}

[reotantan]

何度も申し訳ありません、とてもコードの質が改善しました。
ありがとうございます。
回答者様のご指摘やほかの皆様のおかげで、コードがきれいになったと思います。
ありがとうございました。
#include<iostream>
using namespace std;
int Current_number;
class Bankaccount {
private:
int balance;
int Data_number;
public:
Bankaccount(int ,int);
int getBalance();
void setBalance(int);
int getData_number();
void credit(int);
void debit(int);

};
int Bankaccount::getBalance() {
return balance;
}
void Bankaccount::setBalance(int a) {
balance = a;
}
int Bankaccount::getData_number() {
return Data_number;
}
Bankaccount::Bankaccount(int balanceA,int num) {
if (balanceA <= 0) { // whatever you put into balanceA ,since this is constructor so parameter will be automatically
// be zero
cout << "balance didnt change";
} else {
balance = balanceA; //
Data_number = Current_number;
Current_number++;
}
}
void Bankaccount::credit(int money) {
balance += money;
}
void Bankaccount::debit(int money) {
balance -= money;
}
int main() {
Bankaccount a(3000,0);
a.setBalance(4000);
cout << a.getBalance() << endl;
a.debit(300);
cout << a.getBalance() << endl;
a.credit(1000);
cout << a.getBalance() << endl;
cout<<a.getData_number()<<endl;
Bankaccount b(2000,1);
cout<<b.getData_number();
}

Answer 2

1つの口座に複数のIDが必要になる状況がよくわかりませんが、
IDの発行を管理する何かを用意して、
口座の生成時にはそいつが発行したIDを設定する方法もあります。

あと細かいですが、Data_numberだけスネークケースなのがモヤモヤします。

Comments

[reotantan]

ご指摘の通りです、一つのIDを設定するコードに訂正しました。
ありがとうございました

Answer 3

だいたいChironianさんのご指摘と同じなのですが、それだけでは何ですので、技術的な点というよりは仕様設計として考えてほしい点を指摘させていただきます。

Bankaccountクラスのインスタンスを作成する際に、0以下の値をコンストラクタに渡すと、メンバ変数balanceが0で初期化されます(コードはbalanceAに０を代入していますが、ミスタイプですよね?)。

この時標準出力に"error"と出力されますが、このインスタンスは0で初期化されたものとして引き続き使用することは可能です。だとしたら、error表示はクラスを使用する人を悩ませかねないのではないでしょうか。使用する人というのは何も他人ばかりではなく何日か後の自分自身かもしれません。

もちろん、現在のソースコードはデバッグ段階のもので、リリース版までにはこのあたりをきちんと直されるのだとは思いますが、仕様的に何がエラー(以降の実行を止めるべきもの)で、何がエラーでないか(警告は出すが、問題をプログラム的に回避して以降の処理を継続させる)というものを押さえておくことを心がけるようにしておくと、後々見つけにくい不具合を埋め込んでしまうような事態を回避しやすいと思います。

Comments

[reotantan]

とても参考になりました、ご指摘はとても納得のいくものでした。ありがとうございました