Initialized Variables lose Values after Function Call.

Question

Question says it all, really. I'm not sure what the problem is. I'm fairly new to classes; my practical experience with them being close to nill, but I have read a fair amount about them.

I have created a class ECard with the following constructor

ECard::ECard( int bankNum, int PIN )
{
    m_BankNum     = new int;
    m_PIN         = new int;
    m_Barred      = new bool;
    m_Amount      = new double;

    *m_BankNum     = bankNum;
    *m_PIN         = PIN;
    *m_Barred      = false;
    *m_Amount      = 100.0;
}

and I initialize with EC card( 12345, 54321 )

I also have a member function display(), which simply prints out all the member variables BankNum, PIN, Barred, and Amount.

When I call this function card.display() in my main function, the output is perfectly what I expected.

However, when it enters my loop:

/* Fine values! */
card.display();
while( true )
{
    /* Introductory screen giving user options to choose from */
    mainScreen( card );
    /* Make a choice... */
    choice = readInput();
    /* Garbage! */
    card.display();
    /* Pass it to the switch, watch out for invalid input! doChoice is a bool */
    if( !doChoice( choice, card ) )
    {
        cout << "Bad input- repeat!" << endl;
    }
    /* TODO: Option to terminate loop. */
}

and I try to print the variables in my doChoice() function, I get garbage. All my variables have messed up settings. My Banknumber is something, PIN is a really large negative number( not the MIN ), Barred is suddenly set to true, and there is 0 money in my account even though Barred and Amount were never explicitely set by me anywhere outside the constructor.

/* Outsourced switch that handles the user input from MAIN */
bool doChoice( int choice, ECard card )
{
    int inputPIN;
    int inputAmount;

    /* Garbage! */
    card.display();

    switch( choice )
    {
    case 1:
    /* Case 1: Charge card with money. Needs PIN and amount */
        cout << "PIN Eingeben:  ";
        inputPIN    = readInput();
        cout << "\nBetrag Eingeben:  ";
        inputAmount = readInput();

        karte.aufladen( inputAmount, inputPIN );

        return true;

I'm sorry if some of the member functions and the outputs are still in German. It shouldn't be important anyway( I like to write all my homework in English anyway since I'll probably be programming for international companies, but my teacher is really picky and lowers my grade if I program outside of his expectations )

The variables are already messed up before it even enters the switch, so that part should just be rhetoric. I can see a problem only with the passing of the card object, though I don't know why that would be a problem. I don't know how to fix it, do I just send each member instead? Create a pointer to the object and send that? I mean, I passed the card object around before and the other function didn't give me garbage. Only this one.

On a final note, if there suddenly is an error in syntax, then that is because I quickly translated all my functions and members into English, and I may have missed a capitalization somewhere or whatnot. Please disregard those, the syntax is correct and the program runs, it just displays garbage values.

Answer 1

You are passing card by value. But because you have it full of pointers, and presumably didn't write a copy constructor, the copy of card is using the pointers to change the memory that the "old" card in your main loop is also using. Then when the function exits, presumably you have a card destructor that deletes all the member variables? So you basically have dangling pointers. I can't be sure without all the code for ECard, but that's my guess.

I would not have pointers in card. I see no benefit to an int* over an int in there. And then since you want doChoice to change card, have it take a ECard& so it can change the real one you are using in main.

Answer 2

Is mainScreen() passing 'card' as a reference or by value? What happens is the call creates a new local copy and calls the default constructor, which doesn't allocate memory or initialize the local variables. Same thing is happening with doChoice(). Pass by reference in both those functions and it should fix this problem.

i.e. -- bool doChoice( int choice, ECard &card )

Answer 3

Don't dynamically allocate variables unless necessary

In the Java language, instances of variables are defined using the new operator. This is not necessary in C++.

Change your class declaration to:

struct ECard
{
    ECard(unsigned int bankNum,
          unsigned int newPIN)
    : m_bankNum(bankNum),
      m_pin(newPIN),
      m_barred(false),
      m_amount(100.0)
  {
    ;
  }

  unsigned int m_bankNum; // Can bank numbers be negative???
  unsigned int m_pin;
  bool         m_barred;
  double       m_amount;
};

Pointers

If you really need to dynamically allocate memory for the members, use smart pointers. Smart pointers will automagically deallocate memory memory when needed. I suggest using the Boost library ones.

If you can't use smart pointers, remember to deallocate the memory in the destructor:

ECard::
~ECard()
{
  delete m_p_bankNum;
  delete m_p_pin;
  delete m_p_barred;
  delete m_p_amount;
}

If you don't use pointers in your class or structure, you avoid the hassles of dynamic memory allocation and deallocation, especially for copying and destroying (who owns the memory?).