Common calculator, birth date calculator, units converter, and data recorder

Question

I am very (very) new to programming, and today I (semi-)successfully wrote my first program that performs very basic things.

My hope is that I can receive some constructive criticism around how I structured it all and if I am doing things in the most efficient and proper way possible for what I am trying to achieve.

#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <fstream>
#include <windows.h>
using namespace std;

void calculator();
void birthcal();
void convert();
void readrecorddata();
void recorddata();
void readdata();


int main()
{
for ( int choice = 0;; )
{
    cout << "Please choose one of the following:\n1. Common Calculator\n2. Birth Date Calculator\n3. Convert units of measurement\n4. Read/ Record data to file\n5. Convert Celsius to Fahrenheit\n\nChoice: ";
    cin >> choice;
    cout << endl;

    if (choice == 1)
    {
       calculator();
       break;
    }

    if (choice == 2)
    {
        birthcal();
        break;
    }

    if (choice == 3)
    {
        convert();
        break;
    }

    if (choice == 4)
    {
        readrecorddata();
        break;
    }

    if (choice == 5)
    {
        temperature();
        break;
    }

    else
    {
        cout << "You have made a mistake, please try again. \n";
        cin.clear();
        cin.get();
        system("PAUSE");

        continue;
    }


return 0;
}

}
    void temperature()
    {
    int celsius;
    cout << "Enter the temperature in Celsius: ";
    cin >> celsius;
    int factor;
    factor = 212 - 32;
    int fahrenheit;
    fahrenheit = factor * celsius/100 + 32;
    cout << "Fahrenheit value is: ";
    cout << fahrenheit << endl;
    system("PAUSE");

    }

    void calculator()
    {
    int number1, number2, result;
    for ( ;; )
    {
    cout << "Welcome to Calculator" << endl;
        cout << "Valid operators are +, -, x, /" << endl;
        cout << "Please enter an equation: ";

    char sign(1);

    cin >> number1 >> sign >> number2;


    if (sign == '+')
    {
        result = number1 + number2;
        break;
    }

    if (sign == '-')
    {
        result = number1 - number2;
        break;

    }
    if (sign == 'x')
    {
        result = number1 *  number2;
        break;
    }
    if (sign == '/')
    {
        result = number1 / number2;
        break;
    }

    else
    {
    cout << "You have made a mistake, please try again. \n";
    cin.clear();
    cin.get();
    system("PAUSE");
    continue;
}
    }
        }

    void birthcal()
{
    int age, birthdate, year;
    year = 2014;
    for ( ;; )
{
    cout << "Welcome to the Birth Year calculator" << endl;
    cout << "Please enter your age to determine your birth year: ";
    cin >> age;
    if ( age > 0 && age < 200 )
    {
    birthdate = year - age;

    cout << "\nYou were likely born in the year " << birthdate << " or " << birthdate-- << endl;

    break;
    }

    else
{

    cout << "Invalid age, please try again \n" << endl;
    cin.clear();
    cin.get();
    system("PAUSE");
    continue;
    }
}
}
    void convert()
    {

        int convertchoice;

            for ( convertchoice = 0;; )
            {

        cout << "Welcome to the Units Converter \nPlease pick:\n\n1.Centimeters to Inches \n2.Inches to Centimeters\n\nChoice: ";
        cin >> convertchoice;
        if (convertchoice == 1)
        {
            int cm, in, factor = 2.54;

            cout << "Please enter number in Centimeters: ";
            cin >> cm;

            in = cm / factor;

            cout << "The result is: " << in << endl;

            system("PAUSE");

            break;
            }

        if (convertchoice == 2)
        {

            int cm, in, factor = 2.54;

            cout << "Please enter the number in Inches: ";
            cin >> in;

            cm = in * factor;

            cout << "The result is: " << cm << endl;

            break;
        }

    else
    {


        cout << "You have made a mistake, please try again. \n";
        cin.clear();
        cin.get();
        system("PAUSE");

        continue;
    }

        }
        }

    void readrecorddata()

    {

        for ( int choice = 0;; )
        {
        ofstream myfile;
        cout << "Welcome to Read/ Record Data \n Please select: \n\n1. Record Data\n\n2. Read Data\n\nChoice: ";

        cin >> choice;

        if (choice == 1)
        {
            recorddata();
            break;
        }

        if (choice == 2)
        {
            readdata();
            break;
        }
            else
    {


        cout << "You have made a mistake, please try again. \n";
        cin.clear();
        cin.get();
        system("PAUSE");
        continue;

}

        }}

        void recorddata()
        {
        char name[10], location[15], colour[10], fruit[10];
        int age;

        cout << " - Recording Data - \n\n";

        ofstream myfile;
        myfile.open ("info.txt");
        if (myfile.is_open())
        {
        cout << "Your Name:\n\n";
        cin.getline (name, 10);
        cin >> name;
        myfile << "Name: " << name << endl;
        cout << "\nYour Age:\n\n";
        cin >> age;
        myfile << "Age: " << age << endl;
        cout << "\nCountry:\n\n";
        cin.getline(location, 15);
        cin >> location;
        myfile << "Location: " << location << endl;
        cout << "\nYour favourite colour:\n\n";
        cin.getline(colour, 10);
        cin >> colour;
        myfile << "Favourite Colour: " << colour << endl;
        cout << "\nYour favourite fruit:\n\n";
        cin.getline(fruit, 10);
        cin >> fruit;
        myfile << "Favourite Fruit: " << fruit << endl;

        cout << "/n/nThank You for entering the information";

        system("PAUSE");

        }
        }

        void readdata()
        {
        cout << " - Reading Data - \nNow we will read data from the file.\n\n";
        string line;
        ifstream myfile ("example.txt");
        if (myfile.is_open())
        {
            while ( getline (myfile,line))
            {
                cout << line << '\n';
            }
        }
    }

Answer 1

I have found a couple of things that could help you improve your code.

Use consistent indenting

How your code is formatted has a big impact on how readable it is to others and even to yourself. Pick a style that you find readable and use it consistently.

Add missing temperature() declaration

I couldn't compile your program at first because the declaration of the temperature() routine was missing.

Search for and eliminate unused variables

In the calculator() function, the code sets the value of result but then doesn't do anything with it. I suspect you intended to print the result. Unused variables in code are generally a bad sign, but most compilers will locate them for you and point them out. Learn how to activate that feature of your compiler and use it.

Fix the use of else

In an if .. else clause, the else only "attaches" to the most recent if so in your calculator function, the else at the bottom is only evaluated if the sign is not / which is probably not really what you intended. That whole series of if .. else is probably better expressed as a switch statement instead:

switch (sign) {
case '+':
    result = number1 + number2;
    break;
case '-':
    result = number1 - number2;
    break;
case 'x':
    result = number1 *  number2;
    break;
case '/':
    result = number1 / number2;
    break;
default:
    std::cout << "Sorry, I don't know the " << sign << 
        " operator.\n"
    "I only know operators '+', '-', 'x' and '/'\n";
    std::cin.clear();
    std::cin.get();
}
std::cout << "Result = " << result << '\n';

This same comment applies to almost all the other places you've used a compound if statement, but I'll discuss the whole issue of a menu object below. You'll note that I made a few other changes, which I'll mention below.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Think about your user

Let's say I've typed "5 * 3" into your calculator expecting it to calculate the number 15. Instead the program will tell me "You have made a mistake, please try again." which is not at all helpful and puts the blame on the user rather than the program. I reworded it instead to look like this:

std::cout << "Sorry, I don't know the " << sign << " operator.\n"
    "I only know operators '+', '-', 'x' and '/'\n";

Use appropriate data types

The program uses int data types in a number of places that float or double might be more appropriate. Specifically, the temperature and calculator are arguable cases, but it's definitely the case for your convert function which includes the line:

int cm, in, factor = 2.54;

This doesn't do what you think it does. Specifically, it will create the float literal "2.54" then convert it to the integer "2" and then assign that value to factor. These should be declared float rather than int for the program to make any sense.

Think about the programmer reading your code

Even if nobody else ever looks at your code, there is a benefit in making the structure of the code neat and clear. For instance, in the convert() function, you have the conversion factor defined in two different places and with a fixed number. Does the value change from iteration to iteration? If the number were typed incorrectly, it would have to be changed two places which makes no sense. Finally, it isn't something that should be modified, so it should be declared const. I would probably write it like this:

// number of centimeters per inch, according to the 
// U.S. National Institute of Standards and Technology 
const float CM_PER_INCH = 2.54;

That does three things. It tells what the constant is, tells why the original programmer thinks it should have the value it does and gives a strong hint to experienced programmers that this is a constant by making the name all UPPER CASE. This is an old convention that dates back many decades ago when such things would have been defined in C code using #define because the language didn't originally have const.

Use std::string rather than plain old char[]

In your recorddata function, you have this line:

char name[10], location[15], colour[10], fruit[10];

My full name is longer than 10 characters, so it won't fit. There doesn't seem to be a compelling reason to truncate it since you're using delimiters in the resulting file, so all of these would be better as std::string.

Fix use of std::getline

Your recorddata function currently includes this:

    cout << "Your Name:\n\n";
    cin.getline (name, 10);
    cin >> name;

You don't really want both of those lines. What your program should probably do is to just use the std::cin >> name function and declare name as std::string.

Fix your newline characters

The recorddata function includes this:

cout << "/n/nThank You for entering the information";

Those aren't newline characters at the beginning of that string.

Use a menu object or at least a common menu function

In a number of places in your code, you have something like a menu. Your code presents a couple of options and then asks the user to pick one based on an input number. Rather than repeating that code in many places, it would make sense to make it generic. Only the prompt strings actually change, but the underlying logic of presenting the choices and asking for input are all the same. It looks like you're a beginning programmer, and so perhaps you haven't learned about objects yet, but this kind of repeated task with associated data is really well-suited to object-oriented programming and that's something that C++ is very good at expressing.

Learn the subtle parts of the language

The birthcal routine has a couple of problems, including a fixed, hardcoded year, but I'm only going to point out one of the more subtle errors. The code includes this line:

cout << "\nYou were likely born in the year " << birthdate << " or " << birthdate-- << endl;

It will probably work, but you should know that the result of birthdate-- may be undefined. It's because the order of evaluation within this line is not defined by the standard. Technically we talk about sequence points within the code, and you can read all about it here.

The fix is quite simple here. Just change that to birthdate-1. Also, while you're in there, wouldn't birthyear be a more accurate name for that variable?

Answer 2

A couple of other things:

When you're coding a menu it's much more readable to put each item on a separate line. This also gives you a clearer idea of what the menu will look like when the program runs:

cout << "Please choose one of the following:\n";
cout << "1. Common Calculator\n";
cout << "2. Birth Date Calculator\n";
cout << "3. Convert units of measurement\n";
cout << "4. Read/ Record data to file\n";
cout << "5. Convert Celsius to Fahrenheit\n\n";
cout << "Choice: ";

Whenever you have repeating blocks of code it's good practice to use a sub routine:

void DisplayError(string errormsg)
{
     cout << errormsg << "\n";
     cin.clear();
     cin.get()
}

To replace this code:

cout << "Invalid age, please try again \n" << endl;
cin.clear();
cin.get();

Use:

DisplayError("Invalid age, please try again");

Try to avoid the use of system("PAUSE"). It's considered unsafe since the definition for PAUSE can be redefined outside of the program to do anything.

Answer 3

1. Fix indentation (at any block, indent once)
2. I like infinite while loops instead of infinite for loops (personal preference)
3. I don't believe you need the breaks, you can replace them with an if/else if/else if/else
4. Consider a switch statement instead of the ifs (personal preference)
5. Code safety (validate input parameters)