What can I do to improve this program

[zachdr1]

It's supposed to do tedious math that I don't feel like doing for my engineering classes. Right now it just calculates area.

Here is the main code

 #include <iostream>
#include <vector>
#include "tedious_math_functions.h"

using namespace std;

int main()
{

    char repeat = 'y';

    while (repeat = 'y')
    {
        cout << "Hello, what would like me to do for you? (\"a\" for area of circle)" << endl;

        for(bool flag = false; flag != true;) //used for loop to revert back to if bad input
        {
            char whattodo_char; //what do you want to calculate?
            cin >> whattodo_char;

            switch (whattodo_char)
            {
                case 'a': //calculate area
                {   cout << "Area = " << areaofcircle() << endl;
                    flag = true;
                    break;
                }

                default: {cout << "Sorry, that operation is not supported." << endl;} //wrong operation, reverts back to beginning of for loop
            }
        }

        cout << "Would you like to repeat this?" << endl;
        cin >> repeat;

        if (repeat != 'y') //no repeat, close program
        {
            cout << "Goodbye!" << endl;
            repeat = 'n';
        }

        else
        {
            cout << endl << endl << endl; //reverts back to the beginning of main()
        }
}


    return 0;
}

And here is the header file where my area calculation function is location

#ifndef TEDIOUS_MATH_FUNCTIONS_H_INCLUDED
#define TEDIOUS_MATH_FUNCTIONS_H_INCLUDED
using namespace std;

const double pi = 3.1415926535897;

double areaofcircle() // make this work with more shapes later
{
    cout << "Will you be using the radius or the diameter? (type \"r\" for radius, and \"d\" for diameter)" << endl;

    for(bool flag = false; flag != true;) //makes a loop to where you don't have to keep restarting program if you input the wrong thing
    {
        char radiusordiameter;
        cin >> radiusordiameter;
        double area = 0;

        switch(radiusordiameter) //calculates area depending on if radius or diameter is used
        {

        double in;

        case 'r': //calculates area using radius
            cout << "Input the radius" << endl;
            cin >> in;
            area = pi * in * in;
            return area;

        case 'd': // calculates area using diameter
            cout << "Input the diameter" << endl;
            cin >> in;
            area = pi/4 * in * in;
            return area;

        default: // wrong input
            cout << "You did not tell me if you are using diameter or radius, please try again" << endl;
        }
    }
}

#endif // TEDIOUS_MATH_FUNCTIONS_H_INCLUDED

[AbstractionAnon]

You should not be putting the definition of areaofcircle() in a header file. This will cause problems (duplicate symbol) if you include TEDIOUS_MATH_FUNCTIONS.H in more than one .cpp file.

[jonnin]

if you are going into engineering, I would wire it to solve differential equations...

And I am only half kidding, but what do you find yourself doing a lot of that you would benefit from automating?

[zachdr1]

AbstractionAnon, I thought that was the purpose of header files? So that you can use broad functions in other projects.

jonnin, if only I knew how lol. I was more asking, what can I do to make my code better though. I have heard that there is a huge difference between bad code and good code even if they both get the job done, and I'm afraid that I am writing bad code.

That being said, I found that calculating the area of a circle is extremely annoying when you have to do it 5 times per problem, so I automated that. I'm not sure what else but I'm sure I'll be able to come up with more.

[jonnin]

I don't see anything too bad.

For a starting point
Good code is readable, performs the task in an acceptable time, is easy to add more stuff to.

Bad code is unreadable, slow, or tangled up too much to add more features.

I would use all the digits of pi from the window's calculator, but that is pretty small.

Soon you will take a class on numerical methods, or if its not in the list, TAKE IT.
keep the book. This is the bread and butter of scientific / engineering computing.

[zachdr1]

Nice, that makes me feel pretty good lol, I feel like my code is pretty hard to read but I guess I'm just not fluent enough in c++.

I get to choose 1 math elective to take for my degree in EE, would you say that numerical methods is more important than PDE?

Do you have an engineering or science degree?

[jonnin]

I have a CS degree with a math minor and a focus on scientific computing.

I would honestly bite the bullet and take BOTH. You can't do engineering without PDE and you really want numerical methods if you want to write code to solve problems, which most engineers do NOT do much of, they buy things that do it for them like matlab or whatever else. But a few do it; a guy I worked with was always coding stuff for CFD (computational fluid dynamics) and another was always tinkering with a 6 degree of freedom flight sim.

I worked hand in hand with aerospace engineers for 20 years.

[AbstractionAnon]

I thought that was the purpose of header files?

Common mistake for beginners.
Header files are for shared declarations, not definitions.

What you have in your header file is a definition of the function.
Definitions should go in a .cpp file.

What I would suggest is that you create a TEDIOUS_MATH_FUNCTIONS.cpp file. Put the body (definition) of your shared functions there.

TEDIOUS_MATH_FUNCTIONS.h would still contain the declaration for areaofcircle() and any other functions that you write.

As I mentioned before, if you were to include TEDIOUS_MATH_FUNCTIONS.h (as it exists) in multiple compilation units, areaofcircle() would get compiled multiple times, thereby causing linker errors due to multiple occurrences of the function. The linker would not know which one to include in the generated executable.

BTW, the style of your code is good. It has proper indentation and is easy to read.

One error I do see is line 12 in main. You're using the assignment operator (=), not the equality operator (==). The loop will always execute as true.

Your for loops with a bool work, but seem unconventional. while seems more intuitive.

[jlb]

You should also consider separating the user interface from the actual calculations. Then you can compute the area without being required to get the data directly from the user.

[dhayden]

If you want to automate tedious calculations for your classes then get a programmable calculator and learn to program it. Seriously, you'll be much better off because you can quickly program a formula for a homework problem and useit several times. Being an HP guy, I recommend the new HP Prime.

Now let's talk about your C++ code.

Line 12: = should be ==
I notice that you have lots of code to get a single-letter response. Put that into a function instead:

// Get a one-letter answer to a question from cin.  Keep asking until
// the answer is valid or there is an error.
// "Prompt" is a prompt to print before getting the answer.
// "letter" is the letter that the user entered. This set set when the function
// returns true.
// "validChars" is a list of valid characters. The function keeps prompting until the
// user enters one of these, or until there is an error.
// Returns true on success, false if there was an I/O error.
bool getLetter(const char *prompt, char &letter, const string &validChars)
{
    ...
}

Once you have this, the main program gets much shorter and cleaner. Notice how it flows from "prompt for an answer" into "do something with the answer."

#include <iostream>

using namespace std;

const double pi = 3.1415926535897;

// Get a one-letter answer to a question from cin.  Keep asking until
// the answer is valid or there is an error.
// "Prompt" is a prompt to print before getting the answer.
// "letter" is the letter that the user entered. This set set when the function
// returns true.
// "validChars" is a list of valid characters. The function keeps prompting until the
// user enters one of these, or until there is an error.
// Returns true on success, false if there was an I/O error.
bool
getLetter(const char *prompt, char &letter, const string & validChars)
{
...
}

double
areaofcircle()			// make this work with more shapes later
{
    char radiusOrDiameter;
    double area = 0;
    double in;

    getLetter("Will you be using the radius or the diameter? (type \"r\" for radius, and \"d\" for diameter)",
	      radiusOrDiameter, "rd");

    switch (radiusOrDiameter) {	 //calculates area depending on if radius or diameter is used
    case 'r':				 //calculates area using radius
	cout << "Input the radius" << endl;
	cin >> in;
	area = pi * in * in;
	return area;

    case 'd':				 // calculates area using diameter
	cout << "Input the diameter" << endl;
	cin >> in;
	area = pi / 4 * in * in;
	return area;
    }
}

int
main()
{

    char repeat = 'y';
    char whattodo_char;	//what do you want to calculate?

    do {
	getLetter("Hello, what would like me to do for you? (\"a\" for area of circle)", whattodo_char, "a");
	switch (whattodo_char) {
	case 'a':				 //calculate area
	    {
		cout << "Area = " << areaofcircle() << endl;
		break;
	    }

	}
    } while (getLetter("Would you like to repeat this?", repeat, "yn") && repeat == 'y');

    cout << "Goodbye!" << endl;

    return 0;
}