Thread: Newbie program - improvements?

  1. #1
    Registered Usurer
    Join Date
    Apr 2005
    Location
    upstate NY
    Posts
    79

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

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    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.

  3. #3
    Registered User
    Join Date
    Jun 2005
    Location
    Sarajevo, Bosnia and Herzegovina
    Posts
    18
    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;
    	}}
    
    }

  4. #4
    Registered User
    Join Date
    Jun 2005
    Location
    Sarajevo, Bosnia and Herzegovina
    Posts
    18
    Put this anywhere between int main() and while(n<6)
    Code:
    cout<<"##### ##### ##### #    # ##### ##### ##### #   #\n";
    	cout<<"#     #     #   # ##  ## #       #   #   # #   #\n";
    	cout<<"#  ## ##### #   # # ## # #####   #   ##### #####\n";
    	cout<<"#   # #     #   # #    # #       #   ###       #\n";
    	cout<<"##### ##### ##### #    # #####   #   #  ## ##### CALCULATOR\n\n";

  5. #5
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    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.

  6. #6
    Registered User
    Join Date
    Jun 2005
    Location
    Sarajevo, Bosnia and Herzegovina
    Posts
    18
    Thanks, sure I will work on checking errors.

  7. #7
    Registered User
    Join Date
    Jun 2005
    Location
    Sarajevo, Bosnia and Herzegovina
    Posts
    18
    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");
    }

  8. #8
    Registered User
    Join Date
    Jun 2004
    Posts
    722
    don't use system("pause");

  9. #9
    Registered User
    Join Date
    Jun 2005
    Location
    Sarajevo, Bosnia and Herzegovina
    Posts
    18
    Why ?

  10. #10
    *this
    Join Date
    Mar 2005
    Posts
    498
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Dikumud
    By maxorator in forum C++ Programming
    Replies: 1
    Last Post: 10-01-2005, 06:39 AM
  2. newbie looking for hints on first audio program...
    By BrownB in forum Game Programming
    Replies: 2
    Last Post: 07-13-2005, 07:25 AM
  3. newbie needs help with C++ program
    By cyba in forum C++ Programming
    Replies: 9
    Last Post: 06-25-2004, 02:41 AM
  4. Replies: 2
    Last Post: 05-10-2002, 04:16 PM
  5. My program, anyhelp
    By @licomb in forum C Programming
    Replies: 14
    Last Post: 08-14-2001, 10:04 PM