-
[Advice Needed] Noob Code!
Hello all, I recently began to learn C++ from reading a book. I'm up to chapter 4 of 24 and thought I'd like to try out what I've learnt to far. My program works fine, however I'd like some advice on the way I have written the program and whether or not it can be simplified and how. The program overall is to work of the area of some shapes. The user needs to input the shape he is using the the information required to work out the area.
Code:
#include <iostream>
int AreaTrianlge(short int Height, short int Base)
{
std::cout << std::endl << "Working out the Area of the Triangle!" << std::endl << "The Answer is: ";
return((Height*Base)/2);
}
int AreaSquare(short int Height, short int Width)
{
std::cout << std::endl << "Working out the Area of the Sqaure!" << std::endl << "The Answer is: ";
return(Height*Width);
}
float AreaCircle(float Radius)
{
float PI = 3.14159;
std::cout << std::endl << "Working out the Area of the Circle!" << std::endl << "The Answer is: ";
return(PI*(Radius*Radius));
}
int AreaTrapezium(short int Height, short int TopParallel, short int BottomParallel)
{
std::cout << std::endl << "Working out the Area of the Trapezium!" << std::endl << "The Answer is: ";
return((Height*(TopParallel+BottomParallel))/2);
}
int main()
{
short int ChoosenShape;
short int Height, Base, Width, TopParallel, BottomParallel; //Variables for Area
float Radius; //Variable for Area
std::cout << "Please choose a shape:" << std::endl << "1.Triangle, 2.Square, 3.Circle or 4.Trapezium" << std::endl << "Do this by typing the number of the shape" << std::endl;
std::cin >> ChoosenShape;
if (ChoosenShape == 1)//If Triangle is choosen
{
std::cout << std::endl << "You have choosen 'Triangle'" << std::endl;
std::cout << "Please enter in the height of the Triangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Traingle:\t";
std::cin >> Base;
std::cout << AreaTrianlge(Height, Base) << std::endl;
}
else if (ChoosenShape == 2)//If Square is choosen
{
std::cout << std::endl << "You have choosen 'Square'" << std::endl;
std::cout << "Please enter in the height of the Square:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Square:\t\t";
std::cin >> Width;
std::cout << AreaSquare(Height, Width) << std::endl;
}
else if (ChoosenShape == 3)//If Circle is choosen
{
std::cout << std::endl << "You have choosen 'Circle'" << std::endl;
std::cout << "Please enter in the radius of the Circle:\t";
std::cin >> Radius;
std::cout << AreaCircle(Radius) << std::endl;
}
else if (ChoosenShape == 4)//If Trapezium is choosen
{
std::cout << std::endl << "You have choosen 'Trapezium'" << std::endl;
std::cout << "Please enter in the height of the Trapezium:\t\t\t\t";
std::cin >> Height;
std::cout << "Please enter in the Length of the Top Parallel of the Trapezium:\t";
std::cin >> TopParallel;
std::cout << "Please enter in the Length of the Bottom Parallel of the Trapezium:\t";
std::cin >> BottomParallel;
std::cout << AreaTrapezium(Height, TopParallel, BottomParallel) << std::endl;
}
else
std::cout << std::endl << "Please enter a valid shape number!" << std::endl;
return 0;
}
Like I said before and advice on how I written the could would be amazing thanks all! P.S If this isn't the sort of post that is allowed here, then please close the thread.
-
Re: [Advice Needed] Noob Code!
I'm not crazy about the way you've done the output. You should remove all your output statements from the functions that calculate area. Put them in main with the rest of them.
-
Re: [Advice Needed] Noob Code!
Alright, thanks for the reply. Is
Code:
int AreaTrianlge(short int Height, short int Base)
{
return((Height*Base)/2);
}
more what you mean?
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
Kegs
Alright, thanks for the reply. Is
Code:
int AreaTrianlge(short int Height, short int Base)
{
return((Height*Base)/2);
}
more what you mean?
Yep, except int may not be the best choice for a return value. If Height * Base is an odd number, the result will be wrong.
-
Re: [Advice Needed] Noob Code!
Rather than use multiple nested if statements, you could use the switch statement (though this might be covered in a later chapter in your book). Basically you code as
Code:
switch (ChoosenShape) {
case 1:
std::cout << std::endl << "You have choosen 'Triangle'" << std::endl;
std::cout << "Please enter in the height of the Triangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Traingle:\t";
std::cin >> Base;
std::cout << AreaTrianlge(Height, Base) << std::endl;
break;
case 2:
......
case 3:
......
default:
std::cout << std::endl << "Please enter a valid shape number!" << std::endl;
break;
}
You can also cut down on typing and make your program slightly easier to read by removing all the std:: and inserting using namespace std; after your #include.
Code:
#include <iostream>
using namespace std;
...
cout << "Please choose a shape:" << endl << "1.Triangle, 2.Square, 3.Circle or 4.Trapezium" << endl << "Do this by typing the number of the shape" << endl;
cin >> ChoosenShape;
Also where the result of a calculation is not necessarily an integer (such as area of triangle) rather than returning an int, float could be returned.
I agree with GCDEF about not putting output statements in a function. What if you wanted to use these functions when you were writing a graphical program and the console was not available? Or you wanted output to go to a file instead? A function should really do only one thing (but well!) and not have any un-expected side-effects (such as outputing text).
Question for you to consider. Why have you used short int rather than just int?
-
Re: [Advice Needed] Noob Code!
Thank you both for the feedback. I have now removed the output from the functions and included the 'using namespace std;' into the code. I have used a short int instead of a int as on my computer int is bigger than an int so I did this to make the program smaller.
I'll look into the switch thing, but I'd rather wait to learn fully how to use it later on. Thanks for telling me about it though guys :P
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
Kegs
Thank you both for the feedback. I have now removed the output from the functions and included the 'using namespace std;' into the code. I have used a short int instead of a int as on my computer int is bigger than an int so I did this to make the program smaller.
I'll look into the switch thing, but I'd rather wait to learn fully how to use it later on. Thanks for telling me about it though guys :P
Switch is really just a stylistic thing. Either is okay. As to making your program smaller, 40 years ago saving every byte possible was important, at that level, it's not something you need to be concerned with these days.
-
Re: [Advice Needed] Noob Code!
Pleased to be of help. If you want more feedback later, or have questions about using c++ just post some code and ask. Incidentally, what book are you using from which to learn c++ ?
-
Re: [Advice Needed] Noob Code!
Currently I am using. SAMS Teach Yourself C++ in 24 hours. I found it for like £3 so I thought why not. Not sure if it's a good one, but so far it looks decent.
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
Kegs
Thank you both for the feedback. I have now removed the output from the functions and included the 'using namespace std;' into the code. I have used a short int instead of a int as on my computer int is bigger than an int so I did this to make the program smaller.
Personally, I'd advice against putting "using namespace std;" in your code. It doesn't save much typing (which is always a bad reason, by the way), doesn't make your code that much more readable and it can be an annoying habit later on, when you will work more with header files.
Regarding types, you should default to using (unsigned) int when you need an integer type and double if you need a floating point type. Only use float instead of double if you need to save memory (or disk space) or if you have profiled your application and using float calculations is that much faster than using doubles. Otherwise, just go for the extra precision.
One thing that wasn't mentioned before, is that it's bad style to define all your variables at the beginning of a function. You should try to only define variables once you can give them a meaningful initial value.
-
Re: [Advice Needed] Noob Code!
Alright, thanks for the this. However I'm not quite sure I get what you mean. Do you mean I should do this:
Code:
else if (ChoosenShape == 3)//If Circle is choosen
{
double Radius;
std::cout << std::endl << "You have choosen 'Circle'" << std::endl;
std::cout << "Please enter in the radius of the Circle:\t";
std::cin >> Radius;
std::cout << std::endl << "The answer is: ";
std:: cout << AreaCircle(Radius) << std::endl;
}
EDIT: I have tried this out, and for each if statement, I need to reinitialize each double. For example, I do double Height, in the if Triangle bit, but then again in if Square, I need to do double Height again. Is this ok or would it be easier to just do them all in the begging like I did to begin with? Thanks
-
Re: [Advice Needed] Noob Code!
Yes, that's what meant. However, as you have found, this brings about the concept of variable scope which you probably haven't come across yet in your tutorial book (look for a section(s) on scope). Basically, identifiers declared inside a block (between {..}) have block scope. Block scope begins at the identifier's declaration and ends at the terminating }. Outside of this block scope, the variable is not defined. That's why you had to define the variables in each 'bit' of your program. When they were defined at the beginning they were available to all the 'bits' but when defined in a 'bit' they are only available in that 'bit'. As Radius is only appropriate to a circle, for example, then good practice says that Radius should only be defined in the block that processes circle. Also that variables should be either initialised when defined or as soon as possible after. This is to make sure that a variable is not used before its value has been set. Some compliers on some warning levels produce a compile time message about this.
Just a further observation. The height and width of a square are the same. If they are not then it is a rectangle. So the area of a square is just width * width.
Quote:
I'd advise against putting "using namespace std;" in your code ... doesn't make your code that much more readable and it can be an annoying habit later on, when you will work more with header files.
I agree that you don't put 'using ...' in any header file. However, unless you are using multiple namespaces then my personal opinion is that having 'using namespace std;' after the #include in your program and not using std:: everywhere does make the code a lot more readable - especially when later you'll start to use other standard classes like string, vector etc. What does your book have to say on this?
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
2kaud
I agree that you don't put 'using ...' in any header file. However, unless you are using multiple namespaces...
So, what do you do when you introduce, say, a boost library to a cpp file that you created before? Do you remove "using namespace std;" and spend maybe 10 minutes to fix all the compiler errors? Doesn't seem like a good idea to use a coding style that is so fragile to changes.
The problem I meant with using header files is when you have class definitions or function declarations in a header file and the implementation in a cpp file. If the (member) functions have arguments or a return value that is a class in the std namespace, then the signature will look different in the header file and the cpp file. E.g. in the header file you have
Code:
std::string MyFunction(const std::vector<int>& v);
// and in the cpp file you have
string MyFunction(const vector<int>& v) {
//...
}
If instead you don't use the "using namespace" stuff (at least, not at global scope), then the two are the same and you can just copy-paste.
-
Re: [Advice Needed] Noob Code!
Right, so I've had a bit more of free time so I decided to learn some more. I've learnt about basic classes and the switch method which was previously told here. I have a couple of questions:
Firstly, is this better written code? Is there anything in which I still need to improve on.
Secondly, I'm using Code::Blocks to write my code and when I click 'New' there is an option for class. (I'm not sure if this will be detailed in a later chapter), however would it be possible to create my Class there and refer to it in my main.cpp?
Code:
#include <iostream>
class FindingArea
{
public:
double Height, Base, Radius, TopParallel, BottomParallel;
double Triangle(double Height, double Base)
{
return((Height*Base)/2);
}
double Rectangle(double Height, double Base)
{
return(Height*Base);
}
double Circle(double Radius)
{
return(PI*(Radius*Radius));
}
double Trapezium(double Height, double TopParallel, double BottomParallel)
{
return((Height*(TopParallel+BottomParallel))/2);
}
private:
double PI = 3.14159;
};
int main()
{
int ChoosenShape;
FindingArea Area;
std::cout << "Please choose a shape: " << std::endl << "1.Triangle, 2.Rectangle, 3.Circle, 4.Trapezium or 5. Parallelogram." <<
std::endl << "Do this by typing the number of the shape." << std::endl;
std::cin >> ChoosenShape;
switch (ChoosenShape) {
case 1: //If Triangle is choosen
double Height, Base;//Variables for Triangle
std::cout << std::endl << "You have choosen 'Triangle'" << std::endl;
std::cout << "Please enter in the height of the Triangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Traingle:\t";
std::cin >> Base;
std::cout << std::endl << "The answer is: ";
std::cout << Area.Triangle(Height, Base) << std::endl;
break;
case 2: //If Square is choosen
double Width;//Variables for Square
std::cout << std::endl << "You have choosen 'Rectangle'" << std::endl;
std::cout << "Please enter in the height of the Rectangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Rectangle:\t\t";
std::cin >> Width;
std::cout << std::endl << "The answer is: ";
std::cout << Area.Rectangle(Height, Width) << std::endl;
break;
case 3: //If Circle is choosen
double Radius;//Varable for Circle
std::cout << std::endl << "You have choosen 'Circle'" << std::endl;
std::cout << "Please enter in the radius of the Circle:\t";
std::cin >> Radius;
std::cout << std::endl << "The answer is: ";
std:: cout << Area.Circle(Radius) << std::endl;
break;
case 4: //If Trapezium is choosen
double TopParallel, BottomParallel;//Varaibles for Trapezium
std::cout << std::endl << "You have choosen 'Trapezium'" << std::endl;
std::cout << "Please enter in the height of the Trapezium:\t\t\t\t";
std::cin >> Height;
std::cout << "Please enter in the Length of the Top Parallel of the Trapezium:\t";
std::cin >> TopParallel;
std::cout << "Please enter in the Length of the Bottom Parallel of the Trapezium:\t";
std::cin >> BottomParallel;
std::cout << std::endl << "The answer is: ";
std::cout << Area.Trapezium(Height, TopParallel, BottomParallel) << std::endl;
break;
case 5: //If Parallelogram is choosen
std::cout << std::endl << "You have choosen 'Parallelogram'" << std::endl;
std::cout << "Please enter in the height of the Parallelogram:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Parallelogram:\t\t";
std::cin >> Width;
std::cout << std::endl << "The answer is: ";
std::cout << Area.Rectangle(Height, Width) << std::endl;
break;
default:
std::cout << std::endl << "Please enter a valid shape number!" << std::endl;
break;
}
return 0;
}
Thanks for any information and feedback you have given and will give. :)
-
Re: [Advice Needed] Noob Code!
The main problem I have with your class is that it doesn't make much sense to me. I understand what you are doing with the class, but it's backwards. Your class does one thing, and that's find the area of some shapes. Now, typically, you'd make a basic shape class. From this class you can create various child/derived classes such as: Triangle, Rectangle, Circle, Trapezium, etc. Each of these classes would derive from your base shape class. Then, each of these classes would have their own area function (as well as other functions). Within main you'd get the type of shape, the basic data, and create an object for that shape and then just call its area function.
As for Code::Blocks, creating a new class that way will add two new files to your project. You'll have a .h file where you define your class, then you'll have .cpp file where you actually implement the class. Once you get that done, you can then include the header for the class in your main.cpp file and use the class accordingly.
-
Re: [Advice Needed] Noob Code!
Alright, I'll go back over it next time. I see what you mean, but I haven't really thought that far ahead in my program. I just made the class to really practice it and don't really know what else I can do with the shape. I was thinking of making it either choose between 3-D or 2-D, the leading on to Volume, Density, Surface Area etc. But I feel this may make my program a little messy so I wanted to include each within a class of a seperate file and now that I know it's possible i'll have to have a little play around and post my results. But I see what you mean, like when I call Volume.Sphere or Density.Sphere etc, it could be Sphere.Volume or Sphere.Density.
Thanks again.
-
Re: [Advice Needed] Noob Code!
In the class the way you currently have it, you don't need to define the double variables as you are passing them in as parameters to the class methods.
Have you compiled this program? because
Code:
private:
double PI = 3.14159;
isn't allowed in ANSI c++. You can't initialise a class member like this. You do it via class constructors (or declare as static and initialise at file level). In your case you could do
Code:
private:
double PI;
public:
FindingArea() : PI(3.14159){};
which initialises PI when the default constructor is called (ie when you declare an object to be of class FindingArea).
Now that you have used the switch statement, your variable declarations could do with some attention. As coded, the Height and Base variables defined for case 1 are now common across all the cases as no new block structure is used to limit the scope of the variables as was the case with the if statements. I suggest that either you put them back at the beginning of the program or use a block structure around the code in the case statements such as
Code:
case 1: //If Triangle is choosen
{
double Height, Base;//Variables for Triangle
std::cout << std::endl << "You have choosen 'Triangle'" << std::endl;
std::cout << "Please enter in the height of the Triangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Traingle:\t";
std::cin >> Base;
std::cout << std::endl << "The answer is: ";
std::cout << Area.Triangle(Height, Base) << std::endl;
}
break;
and similar for each of the case statements.
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
D_Drmmr
One thing that wasn't mentioned before, is that it's bad style to define all your variables at the beginning of a function. You should try to only define variables once you can give them a meaningful initial value.
I have to disagree with this. If you define your variables where they are first used, you end up having variable definitions all over the place. Now if you want to know the type of a variable, you have to go hunting for its definition instead of just looking at the top of the function. Also, if you use the same name to define variables in different blocks or scopes, the code becomes confusing to read. Some compilers will usually give you a warning if you use a variable in an expression before assigning any value to it and once you become experienced, assigning a value to a variable the first time it is used, becomes second nature.
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
Coder Dave
I have to disagree with this. If you define your variables where they are first used, you end up having variable definitions all over the place.
That's one way of putting it. I'd say you have the variable definition at the place where it is needed.
Quote:
Originally Posted by
Coder Dave
Now if you want to know the type of a variable, you have to go hunting for its definition instead of just looking at the top of the function.
Typically, I'm more interested in the meaning of a variable than the type of a variable. The initial value is important information for the meaning of a variable. Also, by limiting the scope of a variable the meaning becomes clearer, because the context is defined more precisely.
Quote:
Originally Posted by
Coder Dave
Also, if you use the same name to define variables in different blocks or scopes, the code becomes confusing to read. Some compilers will usually give you a warning if you use a variable in an expression before assigning any value to it and once you become experienced, assigning a value to a variable the first time it is used, becomes second nature.
I don't see how using the same variable name in parallel scopes is confusing. The scope of the variable limits the section of code where it has any meaning at all. The smaller that section is, the easier it is to reason about the code. When I read code like this
Code:
void foo(int selection)
{
int count = 0;
if (selection == 0) {
int n = bar();
// ...
}
// ...
}
then I know I can forget about 'n' after the if-block. That's easier than when I have to remember what it means, because it may be used later in the function.
Also, by defining variables as late as possible, you will be able to make more variables const, which makes the code easier to read as well. You don't need to consider anymore whether a variable is changed somewhere, you know it cannot be.
See also chapters 18 and 19 in "101 Rules, Guidelines and Best Practices" by Sutter and Alexandrescu.
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
Coder Dave
I have to disagree with this. If you define your variables where they are first used, you end up having variable definitions all over the place. Now if you want to know the type of a variable, you have to go hunting for its definition instead of just looking at the top of the function. Also, if you use the same name to define variables in different blocks or scopes, the code becomes confusing to read. Some compilers will usually give you a warning if you use a variable in an expression before assigning any value to it and once you become experienced, assigning a value to a variable the first time it is used, becomes second nature.
I'm going to go with D_Drmmr on this. I think it's cleaner to define them when you use them. There's less hunting that way, and any modern IDE can show you the type and where it's defined with a mouse hover or click if you're having trouble finding it.
-
Re: [Advice Needed] Noob Code!
Ok, so guys. I've finally figured out how to call a class from a different file. (I was calling double Triangle(); instead of double Area::Triangle(); )
But now I've also made a class for working out volume. I've set most of it up just some little things I need to think of, for instance the Cross Section of a prism and the base of pyramid but I'm working on them and they're not my problem. My problem is that I want my int main to only include a few lines; instructions and then whether you want volume or area. Then from what you pick to call either FindArea or FindVolume, I've made both of these functions, but I can't for the life of me, seem to get them to be called. Any help or should I just make an if statement and then if Area if chosen do this else do Volume?
EDIT: I've made a Void for FindArea or FindVolume as I don't want them to return a value but rather run a bunch of code if called. If this is any help. Also I'm currently asking for an input of either 1 or 2. If it's 1 then it will call FindArea else it calls FindVolume.
Thanks again, Kegs
-
Re: [Advice Needed] Noob Code!
Where you should define a variable in a program should be documented in the 'house style' guide for c++ programs that every company involved with programming should have. This should include naming conventions, where/when variables should be defined, layout style etc etc. Different programmers have different views on this and it's important that programmers in a company stick to one convention - especially where more than one programmer is working on a project.
-
Re: [Advice Needed] Noob Code!
And there's another reason to prefer defining variables at the point of first use: In the case of non-primitive (i.e. class type) variables, the definition implies default-construction of the object which sometimes may be a rather complex and expensive process, or even may have side effects that are to be taken into account. (In the case of explicit initialization by passing constructor parameters, the implied construction is more obvious, but chances are some of the constructor parameters are values thad hadn't been determined before.)
And to be consistent, I'd recommend to simply make a habit out of that for primitives as well.
-
Re: [Advice Needed] Noob Code!
Quote:
I've made a Void for FindArea or FindVolume as I don't want them to return a value but rather run a bunch of code if called. If this is any help.
Would help more if you posted the code to look at.
-
Re: [Advice Needed] Noob Code!
Alright, there is a lot of it though:
Code:
#include <iostream>
#include "Area.h"
#include "Volume.h"
int main()
{
int ChoosenShapeType;
std::cout << "Please enter in the type of shape you want: " << std::endl;
std::cout << "1. 2-D Shape (Area), 2. 3-D Shape (Volume)" << std::endl;
std::cout << "Do this by typing in the number of the desired choice." << std::endl;
std::cin >> ChoosenShapeType;
if(ChoosenShapeType == 1)
void FindArea();
else
void FindVolume();
return 0;
}
void FindArea()
{
int ChoosenShape;
Area FindArea;
std::cout << "Please choose a shape: " << std::endl << "1.Triangle, 2.Rectangle, 3.Circle, 4.Trapezium or 5. Parallelogram." <<
std::endl << "Do this by typing the number of the shape." << std::endl;
std::cin >> ChoosenShape;
switch (ChoosenShape)
{
case 1://If Triangle is choosen
{
double Height, Base;//Variables for Triangle
std::cout << std::endl << "You have choosen 'Triangle'" << std::endl;
std::cout << "Please enter in the height of the Triangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Traingle:\t";
std::cin >> Base;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Triangle(Height, Base) << std::endl;
break;
}
case 2: //If Square is choosen
{
double Height, Width;//Variables for Square
std::cout << std::endl << "You have choosen 'Rectangle'" << std::endl;
std::cout << "Please enter in the height of the Rectangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Rectangle:\t\t";
std::cin >> Width;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Rectangle(Height, Width) << std::endl;
break;
}
case 3: //If Circle is choosen
{
double Radius;//Varable for Circle
std::cout << std::endl << "You have choosen 'Circle'" << std::endl;
std::cout << "Please enter in the radius of the Circle:\t";
std::cin >> Radius;
std::cout << std::endl << "The answer is: ";
std:: cout << FindArea.Circle(Radius) << std::endl;
break;
}
case 4: //If Trapezium is choosen
{
double Height, TopParallel, BottomParallel;//Varaibles for Trapezium
std::cout << std::endl << "You have choosen 'Trapezium'" << std::endl;
std::cout << "Please enter in the height of the Trapezium:\t\t\t\t";
std::cin >> Height;
std::cout << "Please enter in the Length of the Top Parallel of the Trapezium:\t";
std::cin >> TopParallel;
std::cout << "Please enter in the Length of the Bottom Parallel of the Trapezium:\t";
std::cin >> BottomParallel;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Trapezium(Height, TopParallel, BottomParallel) << std::endl;
break;
}
case 5: //If Parallelogram is choosen
{
double Height, Width;
std::cout << std::endl << "You have choosen 'Parallelogram'" << std::endl;
std::cout << "Please enter in the height of the Parallelogram:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Parallelogram:\t\t";
std::cin >> Width;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Rectangle(Height, Width) << std::endl;
break;
}
default:
{
std::cout << std::endl << "Please enter a valid shape number!" << std::endl;
break;
}
}
}
void FindVolume()
{
}
If you want my classes just ask too.
Also I haven't written anything in Void FindVolume(); as I wanted it to work before I go writing it. Thanks again, you guys have been a massive help.
-
Re: [Advice Needed] Noob Code!
Quote:
Originally Posted by
Kegs
Alright, there is a lot of it though:
Code:
#include <iostream>
#include "Area.h"
#include "Volume.h"
int main()
{
int ChoosenShapeType;
std::cout << "Please enter in the type of shape you want: " << std::endl;
std::cout << "1. 2-D Shape (Area), 2. 3-D Shape (Volume)" << std::endl;
std::cout << "Do this by typing in the number of the desired choice." << std::endl;
std::cin >> ChoosenShapeType;
if(ChoosenShapeType == 1)
void FindArea();
else
void FindVolume();
return 0;
}
void FindArea()
{
int ChoosenShape;
Area FindArea;
std::cout << "Please choose a shape: " << std::endl << "1.Triangle, 2.Rectangle, 3.Circle, 4.Trapezium or 5. Parallelogram." <<
std::endl << "Do this by typing the number of the shape." << std::endl;
std::cin >> ChoosenShape;
switch (ChoosenShape)
{
case 1://If Triangle is choosen
{
double Height, Base;//Variables for Triangle
std::cout << std::endl << "You have choosen 'Triangle'" << std::endl;
std::cout << "Please enter in the height of the Triangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Traingle:\t";
std::cin >> Base;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Triangle(Height, Base) << std::endl;
break;
}
case 2: //If Square is choosen
{
double Height, Width;//Variables for Square
std::cout << std::endl << "You have choosen 'Rectangle'" << std::endl;
std::cout << "Please enter in the height of the Rectangle:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Rectangle:\t\t";
std::cin >> Width;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Rectangle(Height, Width) << std::endl;
break;
}
case 3: //If Circle is choosen
{
double Radius;//Varable for Circle
std::cout << std::endl << "You have choosen 'Circle'" << std::endl;
std::cout << "Please enter in the radius of the Circle:\t";
std::cin >> Radius;
std::cout << std::endl << "The answer is: ";
std:: cout << FindArea.Circle(Radius) << std::endl;
break;
}
case 4: //If Trapezium is choosen
{
double Height, TopParallel, BottomParallel;//Varaibles for Trapezium
std::cout << std::endl << "You have choosen 'Trapezium'" << std::endl;
std::cout << "Please enter in the height of the Trapezium:\t\t\t\t";
std::cin >> Height;
std::cout << "Please enter in the Length of the Top Parallel of the Trapezium:\t";
std::cin >> TopParallel;
std::cout << "Please enter in the Length of the Bottom Parallel of the Trapezium:\t";
std::cin >> BottomParallel;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Trapezium(Height, TopParallel, BottomParallel) << std::endl;
break;
}
case 5: //If Parallelogram is choosen
{
double Height, Width;
std::cout << std::endl << "You have choosen 'Parallelogram'" << std::endl;
std::cout << "Please enter in the height of the Parallelogram:\t";
std::cin >> Height;
std::cout << "Please enter in the base of the Parallelogram:\t\t";
std::cin >> Width;
std::cout << std::endl << "The answer is: ";
std::cout << FindArea.Rectangle(Height, Width) << std::endl;
break;
}
default:
{
std::cout << std::endl << "Please enter a valid shape number!" << std::endl;
break;
}
}
}
void FindVolume()
{
}
If you want my classes just ask too.
Also I haven't written anything in Void FindVolume(); as I wanted it to work before I go writing it. Thanks again, you guys have been a massive help.
If it hasn't been said already, I'd avoid duplicating names. You have an object named FindArea inside a function named FindArea. That typically leads to confusion and errors.
From an OOP perspective your code kind of misses the point. I'm not sure how far you've got in your studies, but this is a typical OOP learning exercise. It's usually implemented by creating a Shape class with virtual functions. You'll derive your shape types from that base class and override the virtual functions so that each shape can calculate its own area, etc.
-
Re: [Advice Needed] Noob Code!
Quote:
It's usually implemented by creating a Shape class with virtual functions.
This is the important OOP concept of polymorphism. Put very simply, calling a base class function via a pointer or reference actually calls the appropriate function in a derived class depending upon to which derived class the base pointer has been set. This is a fairly advanced concept in c++ class design and you may not have met it yet in the book but it's fundamental to the power of OOP. To give a simple example based upon your area program, using OOP you could code it something like
Code:
#include <iostream>
using namespace std;
class shape {
public:
virtual double area() const = 0;
};
class circle : public shape {
private:
double radius;
circle();
public:
circle (double r) : radius(r) {}
double area() const {
return (3.1416 * radius * radius);
}
};
class square : public shape {
private:
double side;
square();
public :
square (double s) : side(s) {}
double area() const {
return (side * side);
}
};
int main()
{
int opt;
shape *sh;
cout << "1) Square area\n2) Circle area\n:";
cin >> opt;
switch (opt) {
case 1:
{
double length;
cout << "Side length: ";
cin >> length;
sh = new square(length);
}
break;
case 2:
{
double rad;
cout << "Radius: ";
cin >> rad;
sh = new circle(rad);
}
break;
default:
cout << "Invalid option" << endl;
return (1);
}
cout << "Area is: " << sh->area() << endl;
return (0);
}
There is only one call to sh->area whether it's for a square or circle. The system figures out which particular area is meant by which class sh is pointed to. This concept would be extended to other plane shapes by creating more classes and having say another method for perimiter. Give it a read in your book and let us know if you have any questions.
-
Re: [Advice Needed] Noob Code!
Thanks for the information guys. However being honest most of it has gone over my head. I think I've jumped in the deep end and tried to be too ambitious, so I'm going to go back over what I've learnt so far, make sure I'm really comfortable with them and then maybe try this again. You guys have been an massive help, thanks a lot. :)
