My first OOP

hi guys, would just like your thoughts on my first object orientated program of a ATM machine. also theres a little prob, it doesnt seem to deduct money from the account.

Code:

---

#include <iostream>
#include <stdlib.h>

using namespace std;

class ATM
{
      private:
      int CardNumber, PinNumber;
      int Balance;
     
      public:
      void EnterCard(); 
      void SetBalance();
      void ShowBalance();
      void DrawMoney();
      void DispenseMoney(int Amount);                                 
};

//------------------------------------------------------------------
ATM ATM1; 
//------------------------------------------------------------------ 
 
void ATM::SetBalance()
{
    cout << "How much do you want in your account? " << endl;
    cin >> ATM1.Balance;
   
    cout << "Balance is now; " << ATM1.Balance << endl;
    system("pause");
}
//-----------------------------------------------------------------         
void ATM::EnterCard()
{
    cout << "Card Number : ";
    cin >> ATM1.CardNumber;
   
    cout << "Pin Number  : ";
    cin >> ATM1.PinNumber;
}
//------------------------------------------------------------------
void ATM::DrawMoney()
{
    int Amount;
    cout << "How much money would you like: ";
    cin >> Amount;
   
    if(Amount > ATM1.Balance)
    {
                cout << "Sorry, you do not have enough funds" << endl;
    }
    else {ATM1.DispenseMoney(Amount);}
}
//--------------------------------------------------------------------
void ATM::DispenseMoney(int Amount)
{
    ATM1.Balance - Amount;
}
//-------------------------------------------------------------------
void ATM::ShowBalance()
{
    cout << "Your balance is: " << ATM1.Balance << endl;
    system("pause");
}
   
//---------------------------------------------------------------
int main(int argc, char *argv[])
{
 int Menu;
 
ATM1.EnterCard();
ATM1.SetBalance();

while(Menu != 3)
{         
 cout << "1) Display Balance, 2) Draw Money out, 3) Exit : ";
 cin >> Menu;
 
 switch(Menu)
{
            case 1:
                  {
                    ATM1.ShowBalance(); 
                    break;       
                  }
            case 2:
                  {
                    ATM1.DrawMoney();
                    break;       
                  }
}

 Menu = 0;
 system("cls");
}//while != 3
 
 return 0; 
}

---

thanks,

Code:

---

ATM1.Balance - Amount; // ATM1.Balance NOT modified

---

Should be:

Code:

---

ATM1.Balance -= Amount;

---

It looks good to me. Though you shouldn't use OS dependent functions like system().
Why do you use this:

Code:

---

//------------------------------------------------------------------

---

Why not use a whitespace?

yes that works, thanks.

not sure, it just seems to seperate it a bit more.

The only think I would consider a problem is that you mix declaration with implementation. Class declarations should be in .h files while the implementations in .cpp files, and main should be in a separate .cpp file and use the header file for your class. Mixing the three leaves no secrets in your program given you have any worth keeping, it's just good programming practice.

Code:

---

#include <stdlib.h>

---

Is better as

Code:

---

#include <cstdlib>

---

sorry to criticise, buy I think that while you're getting the hang of classes, you've haven't grasped the concept of objects.

this code, for instance

Code:

---

ATM ATM1; 
//------------------------------------------------------------------ 
 
void ATM::SetBalance()
{
    cout << "How much do you want in your account? " << endl;
    cin >> ATM1.Balance;
   
    cout << "Balance is now; " << ATM1.Balance << endl;
    system("pause");
}

---

you've essentially designed a system where there can only ever be one ATM! instead of creating a global ATM1 object you class member functions should operate on their own data.

so your code should be

Code:

---

void ATM::SetBalance()
{
    cout << "How much do you want in your account? " << endl;

    cin >> Balance; // note: NO ATM1, use "this->Balance" if you prefer
   
    cout << "Balance is now; " << Balance << endl; // note: NO ATM1, use "this->Balance" if you prefer
    // system("pause"); be gone, foul "system" call!
}

---

Also as a general rule you should separate your user interface from your application logic. so while it's ok to have the ATM class handle your user interface (i.e. cin and cout), you should have a separate Account class that manages balances and so on.

Practically, your code is ok for a simple project this size. But if you develop good coding practices now, it'll make your life much easier later on.