Newbie program - improvements?

Hello all -

I've read C++ for Dummies, and I'm working my way through Accelerated C++ based on the advice of this board.

I created this app from scratch but it's rough, I'm hoping for suggestions on how to improve it. I've got some database, music, and Windows projects in mind using Visual C++.Net after I get the hang of basic programming with DevC++.

Thanks in advance - JM

Code:

---

// Pace Calculator
// Input Minutes and Seconds run
// as well as mileage

// Output is pace per mile and miles per hour


#include<iostream>

#include<cstdlib>

// say what standard-library names we use
using std::cin;              using std::endl;
using std::cout;           

int main()
{
    // print program purpose
    cout<<"Pace Calculator"<<endl<<endl;
   
    // ask for the minutes
    cout<<"Please enter minutes run ";
   
    // read minutes
    int minutes = 0;
    cin>>minutes;
   
    // and seconds
    cout<<"and seconds ";
    int seconds = 0;
    cin>>seconds;
   
    // ask for mileage
    cout<<endl;
    cout<<"What was the mileage? ";
    double mileage = 0;
    cin>>mileage;
   
    //Do the math for pace per mile
    double TotSeconds = 0;
    TotSeconds = minutes * 60 + seconds;
    double PaceSeconds = 0;
    PaceSeconds = TotSeconds/mileage;
    int PaceMinutes = 0;
    PaceMinutes = TotSeconds/mileage/60;
    PaceSeconds = PaceSeconds - PaceMinutes * 60;

    // Print pace per mile
    cout<<endl;
    cout<<"Pace per mile was "<<PaceMinutes<<":"<<PaceSeconds<<endl;
   
    // Calculate MPH
    double MPH = 0;
    MPH = 60*60 / (TotSeconds/mileage);
   
    // Print MPH
    cout<<endl<<"which is "<<MPH<<" MPH"<<endl;
    cout<<endl;   
    system("PAUSE");
    return 0;
   
}

---

Looks very good to me. Minor additions could include error checking on user input. Wrap the cin >> calls in an if or while to check for errors, for example:

Code:

---

if (!(cin >> minutes)) {
  cout << "Bad input... exiting..." << endl;
  return EXIT_FAILURE;
}

---

Or you could put it in a while loop to ask again. You would have to ignore extraneous characters and clear cin of its fail state if you let the user try again.

You could also try moving the calculations to one or more separate functions.

I made too, some little application, so what you think about it and how I can upgrade it (ideas).

Code:

---

// Calculating application
// by: Miljan Sucur

#include <iostream>
using namespace std;
//Function for girth
//using 2 integers and return girth
int obim(int x, int y)
{
        return ((2*x)+(2*y));
}
//Function for area
int povrsina(int x, int y)
{
        return(x*y);
}
//Function for Main menu
void menu()
{
        cout << "\n******** M E N U ********";
        cout << "\nGIRTH\t\t\t(1)\nAREA\t\t\t(2)\nNEW DIMENSIONS\t\t(3)\nDRAW SHAPE\t\t(4)\nCURRENT DIMENSIONS\t(5)\nEXIT\t\t\t(0)\n";
}
//Class for dimensions
class broj
{
public:
        broj();
        ~broj();
        int getDuzina();
        int getSirina();
        void setDuzina(int &sDuzina);
        void setSirina(int &sSirina);
private:
        int Duzina;
        int Sirina;


};
//Constructor make default dimensions to 4 and 6
broj::broj()
{
        Duzina = 4;
        Sirina = 6;
}
broj::~broj()
{}
//Set lenght
void broj::setDuzina(int &sDuzina)
{
        Duzina = sDuzina;
}
//Set width
void broj::setSirina(int &sSirina)
{
        Sirina = sSirina;
}
//Return values
int broj::getDuzina()
{
        return Duzina;
}
int broj::getSirina()
{
        return Sirina;
}
       
//Main function
int main()
{
        broj kocka;
        int n,d,s;
        int a,b;
        n = 0;
        int &mNumber = n;
       
while (n < 6)
{
a = kocka.getDuzina();
b = kocka.getSirina();
        menu();
cout << "\nChoose: ";
cin >> n;

        if (n == 0)
        {cout << "\nEXITING!\n\n\n";
        break;}
switch (n)
case 1:
        {
                cout << "\n************************************\nGirth of shape: " << obim(a,b)<< "\n************************************\n";
        break;
case 2:
       
                cout << "\n************************************\nArea of shape: " << povrsina(a,b)<<"\n************************************\n";
        break;
case 3:
        cout << "\nInput new size for lenght: ";
                cin >> d;
                cout << "Input new size for width: ";       
                cin >> s;
        kocka.setDuzina(d);
        kocka.setSirina(s);
        break;
case 4:
        cout << "\n";
        int i,j;
        for (i=0;i<a;i++)
        {
        for (j=0;j<b;j++)
        cout << "* ";
        cout << "\n";}
        break;
case 5:
        cout << "\nCurrent dimensions: ";
        cout << "\n************************************\nLenght= " << a;
        cout <<"\nWidth= " << b << "\n************************************\n";
        break;
case 0:
        break;
        }}

}

---

Put this anywhere between int main() and while(n<6)

Code:

---

cout<<"##### ##### ##### #    # ##### ##### ##### #  #\n";
        cout<<"#    #    #  # ##  ## #      #  #  # #  #\n";
        cout<<"#  ## ##### #  # # ## # #####  #  ##### #####\n";
        cout<<"#  # #    #  # #    # #      #  ###      #\n";
        cout<<"##### ##### ##### #    # #####  #  #  ## ##### CALCULATOR\n\n";

---

That looks good, too. I would consider moving the user menu choice code into the menu() function and have that function return the response. Then have a different function for each response.

I might also look into using const for your broj class. The set methods should just take a regular int, there's no need for an int reference (it is just as easy to copy the int as it is to copy a reference to the int). The get methods can be const since they don't modify the class instance. This doesn't matter in this program but it is a good habit to get into:

Code:

---

int getDuzina() const;

...

int broj::getDuzina() const
{
  return Duzina;
}

---

You could also add error checking for the input to make sure the user doesn't type a letter or something bad that would essentially crash your program.

Lastly, better code formatting would make it much easier to read.

Thanks, sure I will work on checking errors.

I fixet some yours suggestions, now menu isn't always here (expect first time), you must call him on (6), and checking error is here, if you press unacceptable number you will be returned on the start of main().

Code:

---

// Calculating application
// by: Miljan Sucur

#include <iostream>
using namespace std;

//Function for girth
//using 2 integers and return girth
int obim(int x, int y)
{
        return ((2*x)+(2*y));
}

//Function for area
int povrsina(int x, int y)
{
        return(x*y);
}

//Function for Main menu
void menu()
{
        cout << "\n******** M E N U ********";
        cout << "\nGIRTH\t\t\t(1)\nAREA\t\t\t(2)\nNEW DIMENSIONS\t\t(3)";
        cout << "\nDRAW SHAPE\t\t(4)\nCURRENT DIMENSIONS\t(5)";
        cout << "\nMENU\t\t\t(6)\nEXIT\t\t\t(0)\n";
}

//Class for dimensions
class broj
{
public:
        broj();
        ~broj();
        int getDuzina();
        int getSirina();
        void setDuzina(int &sDuzina);
        void setSirina(int &sSirina);
private:
        int Duzina;
        int Sirina;


};
//Constructor make default dimensions to 4 and 6
broj::broj()
{
        Duzina = 4;
        Sirina = 6;
}
broj::~broj()
{}
//Set lenght
void broj::setDuzina(int &sDuzina)
{
        Duzina = sDuzina;
}
//Set width
void broj::setSirina(int &sSirina)
{
        Sirina = sSirina;
}
//Return values
int broj::getDuzina()
{
        return Duzina;
}
int broj::getSirina()
{
        return Sirina;
}
       
//Main function
int main()
{
        cout<<"##### ##### ##### #    # ##### ##### ##### #  #\n";
        cout<<"#    #    #  # ##  ## #      #  #  # #  #\n";
        cout<<"#  ## ##### #  # # ## # #####  #  ##### #####\n";
        cout<<"#  # #    #  # #    # #      #  ###      #\n";
        cout<<"##### ##### ##### #    # #####  #  #  ## ##### CALCULATOR\n\n";
        broj kocka;
        int n,d,s;
        int a,b;
        n = 0;
        int &mNumber = n;
        menu();
       
while (n < 7)
{
a = kocka.getDuzina();
b = kocka.getSirina();
cout << "\nChoose: ";
cin >> n;

        if (n == 0)
        {cout << "\nEXITING!\n\n\n";
        break;}
switch (n)
case 1:
        {
                cout << "\n************************************\nGirth of shape:";
                cout << " " << obim(a,b)<< "\n************************************\n";
        break;
case 2:
       
                cout << "\n************************************\nArea of shape:";
                cout << " " << povrsina(a,b)<<"\n************************************\n";
        break;
case 3:
        cout << "\nInput new size for lenght: ";
                cin >> d;
                cout << "Input new size for width: ";       
                cin >> s;
        kocka.setDuzina(d);
        kocka.setSirina(s);
        break;
case 4:
        cout << "\n";
        int i,j;
        for (i=0;i<a;i++)
        {
        for (j=0;j<b;j++)
        cout << "* ";
        cout << "\n";}
        break;
case 5:
        cout << "\nCurrent dimensions: ";
        cout << "\n************************************\nLenght= " << a;
        cout <<"\nWidth= " << b << "\n************************************\n";
        break;
case 6:
        menu();
        break;
case 0:
        break;
default:
        cout << "\n********************************************************\n";
        cout << "Wrong solution, please try again (call menu, press (6))\n";
        cout << "********************************************************\n\n";
        return main();
        }}
system("PAUSE");
}

---

don't use system("pause");

Why ?

Read the FAQ, it's bad because it spawns another process to do the work for you, which could potentialy be something you dont want it to be, ie a virus etc...

It's not secure. It's not portable.

There are other methods that are much better, search the FAQ or the boards.