c++ classes member functions

Question

With the following code, where the member functions are declared in the class, it prompts the user to enter various inputs like hours and pay.

cout << endl << "Now please input info #1" << endl;
Info p1;
cout << endl << "Now please input info #2" << endl;
Info p2;
p2.combineinfo(p1);  /**< combines the info, like hours, pay, ect.  */
printinfo(pnew);  /**< prints out the combined paycheck information  */

The information is put into p1 and p2 accurately because I can confirm with a cout. However, p2.combineinfo(p1) prompts again for a set of information inputs. I don't want this, I just need it to pass to this function to get combined and then be printed out with printinfo();.

Info Info::combineInfo(Info p1)
{
Info p2;
Info pnew;
pnew.name = p1.name;
pnew.hours = p1.hours + p2.hours;
pnew.total = p1.total + p2.total;
return pnew;
}

Updated Info:

Info::Info()
{
string dummy;
cout << "question here"<<endl;
getline(cin, a);
cout <<"question here"<<endl;
cin >> b;
getline(cin, dummy);
cout <<"question here"<<endl;
cin >> c;
getline(cin, dummy);
cout << "quesiton here"<< endl;
initializeDate(start);
cout << "question here "<< endl;
initializeDate(finish);
}

Answer 1

Since I ended up typing a lot of comments, I've combined it into an answer:

As per the other answers, your issue is that you are calling Info's constructors in your combineInfo function, and you are prompting the user in the constructors. It's usually a bad idea to use std::cin inside a constructor - What if you (or your code's users) decide to use the code in a situation where std::cin is inappropriate (e.g. when you later decide to upgrade to a GUI)? What if you want to construct the object with pre-determined data? Or in a unit test?

In fact your own code provides just such an example: you've defined Info in a way that the only way it can be constructed is by asking for info from the user. However, you have two ways that you actually want to construct the object - (1) by prompting for info from the user, and (2) by combining info which already exists inside two other Info objects (p1 and p2).

The better solution is to have the constructor receive the data via parameters - Info::Info(string name, int hours, int total), obtain the info from the user outside the class (when appropriate), and pass it in to the constructor.

Alternatively, if you're determined to keep the cin code inside the constructor, then do not use a compareInfo() function - make a second constructor which takes two Info objects as parameters, and place your code there:

Info::Info(Info p1, Info p2)
{
    name = p1.name;
    hours = p1.hours + p2.hours;
    total = p1.total + p2.total;
}

Then do your combining like this:

Info pnew(p1, p2);

EDIT: you've commented that have a criterion you must stick to, namely that the combining must happen inside a Info Info::combineInfo(Info) function. This is awkward because your constructor prompts for information, and you must construct a temporary Info object inside your member function:

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;
}

You have three solutions that I can think of, none one of them particularly attractive (and I am surprised that a teacher would impose this requirement on you):

(1) Provide a constructor which does not prompt for info. However it must have a different signature to the one which does request info, because you cannot define a default constructor twice:

Info::Info()
{
   // Prompt user.
}

Info::Info(int dummyValue)
{
   // Do nothing.
}

Then when you create your pnew object, call the second constructor: Info pnew(0);.

(2) Do not provide a default constructor at all, instead create one which you inform whether or not to prompt:

Info::Info(bool prompt)
{
    if (prompt)
    {
        // Construct object by prompting user.
    }
    // No need to do an else, simply do nothing if !prompt
}

This has the disadvantage that you will always have to supply a parameter to construct Info objects:

Info p1; // Error, no default constructor
Info p1(true); // Construct by prompting
Info pnew(false); // Construct without prompting

(3) Do not create a temporary pnew at all, just modify p1 and return that. This is poor style, and only works because you have passed p1 by value (therefore you are not modifying the original 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;
}

---

Other comments:

You also have a couple of bugs in your code as it stands:

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.

Change the code to this:

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.

The second bug is as follows:

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;
}

This only seems to work correctly for you, because of the constructor issue - you're being prompted to enter the info for combineInfo's p2, so it ends up being the same as the p2 you actually intended to use (assuming you entered the same data again), and it seems to combine them correctly. Do this instead:

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 and total in this case are shorthand for this.hours and this.total.

Another thing to consider for future reference, is to overload the + operator instead of writing a combine function:

Info Info::operator+(Info p1)
{
Info pnew;
pnew.name = p1.name;
pnew.hours = p1.hours + hours;
pnew.total = p1.total + total;
return pnew;
}

Now instead of:

Info pnew = p2.combineInfo(p1);

You can write:

Info pnew = p1 + p2;

Finally, consider passing by reference instead of by value - for non-trivial objects it's usually faster for the function to dereference the address instead of making a new copy of it:

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.

EDIT: But don't do this if you use solution 3 above, otherwise you will modify the original p1 which is probably not desireable to you.

Answer 2

Presumably you have decided have the default constructor for Info read in the value. Then in the method combineInfo you again have a couple of constructors which must do the same.

You can avoid the problem by having a method after construction that prompts for the data. If you show your constructor code that might help.

Having said that, most likely the method ought to read:

void Info::combineInfo(const Info& p1)
{
    name = p1.name; // Why not keep p2.name?
    hours += p1.hours;
    total += p1.total;
}

and then call:

printInfo(p2);

Answer 3

i'm guessing you're reading in input froms stdin inside your constructor for Info (could you post this code?). thus, when you create p2 and pnew again, it's calling that very same code.

i suggest you have separate code for initializing a variable with stdin, instead of your constructor. otherwise, there's not much you can do here about it being called!

Answer 4

You don't show it but you are probably asking for the input in the Info constructor. Something like:

Info::Info()
{
   cout << "Enter name: ";
   cin >> name;
   cout << "Enter hours: ";
   cin >> hours;
   cout << "Enter total: ";
   cin >> total;
}

So in combineInfo you create two objects of class Info, so the constructors run and it asks for input for each one. Just don't ask for the values in the constructor. Add another method AskForInput to ask for the values and don't call it from the constructor.