たくさんのコメントを入力することになったので、それを組み合わせて答えにしました。
他の回答と同様に、問題は、関数でInfo
のコンストラクターを呼び出しており、コンストラクターでユーザーにプロンプトを表示していることです。combineInfo
通常、std::cin
コンストラクター内で使用することはお勧めできません-あなた(またはコードのユーザー)std::cin
が不適切な状況(後でGUIにアップグレードすることを決定した場合など)でコードを使用することにした場合はどうなりますか?事前に決定されたデータを使用してオブジェクトを構築する場合はどうなりますか?またはユニットテストで?
実際、独自のコードはまさにそのような例を提供します。Info
それを構築できる唯一の方法は、ユーザーに情報を求めることであるという方法で定義しました。ただし、実際にオブジェクトを作成する方法は2つあります。(1)ユーザーからの情報の入力を求める方法と、(2)他の2つのオブジェクト(および)内にすでに存在する情報を組み合わせることです。Info
p1
p2
より良い解決策は、コンストラクターにパラメーターを介してデータを受信させInfo::Info(string name, int hours, int total)
、クラス外のユーザーから情報を取得し(適切な場合)、それをコンストラクターに渡すことです。
または、コンストラクター内にコードを保持することにした場合は、関数cin
を使用しないでください。2つのオブジェクトをパラメーターとして受け取る2番目のコンストラクターを作成し、そこにコードを配置します。compareInfo()
Info
Info::Info(Info p1, Info p2)
{
name = p1.name;
hours = p1.hours + p2.hours;
total = p1.total + p2.total;
}
次に、次のように組み合わせます。
Info pnew(p1, p2);
編集:あなたはあなたが固執しなければならない基準を持っているとコメントしました、すなわち、結合はInfo Info::combineInfo(Info)
関数の中で起こらなければなりません。Info
コンストラクターが情報の入力を求め、メンバー関数内に一時オブジェクトを作成する必要があるため、これは厄介です。
Info Info::combineInfo(Info p1)
{
Info pnew; // Temporary Info object
pnew.name = p1.name;
pnew.hours = p1.hours + hours;
pnew.total = p1.total + total;
return pnew;
}
あなたには私が考えることができる3つの解決策がありますが、どれも特に魅力的ではありません(そして教師があなたにこの要件を課すことに驚いています):
(1)情報の入力を求めないコンストラクターを提供します。ただし、デフォルトのコンストラクターを2回定義することはできないため、情報を要求するものとは異なる署名が必要です。
Info::Info()
{
// Prompt user.
}
Info::Info(int dummyValue)
{
// Do nothing.
}
次に、pnew
オブジェクトを作成するときに、2番目のコンストラクターを呼び出しますInfo pnew(0);
。
(2)デフォルトのコンストラクターをまったく提供せず、代わりに、プロンプトを表示するかどうかを通知するコンストラクターを作成します。
Info::Info(bool prompt)
{
if (prompt)
{
// Construct object by prompting user.
}
// No need to do an else, simply do nothing if !prompt
}
これには、Infoオブジェクトを作成するために常にパラメータを指定する必要があるという欠点があります。
Info p1; // Error, no default constructor
Info p1(true); // Construct by prompting
Info pnew(false); // Construct without prompting
(3)一時的なものを作成するのpnew
ではなく、変更p1
して返すだけです。これは貧弱なスタイルであり、p1
値を渡したためにのみ機能します(したがって、元のスタイルを変更していませんp1
)。
Info Info::combineInfo(Info p1)
{
// No need to do anything with name since it's p1's name you want anyway
p1.hours += hours;
p1.total += total;
return p1;
}
他のコメント:
また、現状のコードにはいくつかのバグがあります。
cout << endl << "Now please input info #1" << endl;
Info p1;
cout << endl << "Now please input info #2" << endl;
Info p2;
p2.combineinfo(p1); // You are not saving the return value.
printinfo(pnew); // pnew does not exist, see above.
コードを次のように変更します。
cout << endl << "Now please input info #1" << endl;
Info p1;
cout << endl << "Now please input info #2" << endl;
Info p2;
Info pnew = p2.combineinfo(p1); // Now you are saving the result.
printinfo(pnew); // pnew now exists.
2番目のバグは次のとおりです。
Info Info::combineInfo(Info p1)
{
Info p2; // You're creating a new p2 object, this is
// NOT the p2 you called the function on.
Info pnew;
pnew.name = p1.name;
pnew.hours = p1.hours + p2.hours; // Wrong p2
pnew.total = p1.total + p2.total; // Wrong p2
return pnew;
}
コンストラクターの問題があるため、これは正しく機能しているようcombineInfo
に見えます-の情報を入力するように求めp2
られているため、p2
実際に使用しようとしているものと同じになります(同じデータを再度入力したと仮定します) )、そしてそれはそれらを正しく組み合わせているようです。代わりにこれを行ってください:
Info Info::combineInfo(Info p1)
{
// Info p2; // Delete this line
// your intended p2 is passed to the function as the implicit parameter.
Info pnew;
pnew.name = p1.name;
pnew.hours = p1.hours + hours; // hours refers to implicit parameter
pnew.total = p1.total + total; // total refers to implicit parameter
return pnew;
}
hours
total
この場合、this.hours
との省略形ですthis.total
。
将来の参照のために考慮すべきもう1つのことは+
、結合関数を作成する代わりに、演算子をオーバーロードすることです。
Info Info::operator+(Info p1)
{
Info pnew;
pnew.name = p1.name;
pnew.hours = p1.hours + hours;
pnew.total = p1.total + total;
return pnew;
}
今の代わりに:
Info pnew = p2.combineInfo(p1);
あなたは書ける:
Info pnew = p1 + p2;
最後に、値ではなく参照で渡すことを検討してください。重要なオブジェクトの場合、関数がアドレスの新しいコピーを作成する代わりに、アドレスを逆参照する方が通常は高速です。
Info Info::combineInfo(const Info& p1); // p1 passed by reference
// const replicates the behaviour
// of pass by value whereby
// you cannot accidentally modify
// the original object.
編集:しかし、上記の解決策3を使用する場合は、これを行わないでください。そうしないと、p1
おそらく望ましくないオリジナルを変更することになります。