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

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
#include
#include
#include
#include
using namespace std;

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

int main()
{
for ( int choice = 0;; )
{
cout > choice;
cout > celsius;
int factor;
factor = 212 - 32;
int fahrenheit;
fahrenheit = factor * celsius/100 + 32;
cout > 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 > age;
if ( age > 0 && age > convertchoice;
if (convertchoice == 1)
{
int cm, in, factor = 2.54;

cout > cm;

in = cm / factor;

cout > in;

cm = in * factor;

cout > choice;

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

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

cout > name;
myfile > age;
myfile > location;
myfile > colour;
myfile > 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;
ifstre

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 > 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 str

StackExchange Code Review Q#49390, answer score: 3