[Advice Needed] Noob Code! - Page 2
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 2 of 2 FirstFirst 12
Results 16 to 28 of 28

Thread: [Advice Needed] Noob Code!

  1. #16
    Join Date
    Feb 2013
    Posts
    10

    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.

  2. #17
    Join Date
    Dec 2012
    Location
    England
    Posts
    2,460

    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.

  3. #18
    Join Date
    Feb 2013
    Location
    United States
    Posts
    56

    Re: [Advice Needed] Noob Code!

    Quote Originally Posted by D_Drmmr View Post
    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.

  4. #19
    Join Date
    Jul 2005
    Location
    Netherlands
    Posts
    2,013

    Re: [Advice Needed] Noob Code!

    Quote Originally Posted by Coder Dave View Post
    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 View Post
    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 View Post
    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.
    Cheers, D Drmmr

    Please put [code][/code] tags around your code to preserve indentation and make it more readable.

    As long as man ascribes to himself what is merely a posibility, he will not work for the attainment of it. - P. D. Ouspensky

  5. #20
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Posts
    12,108

    Re: [Advice Needed] Noob Code!

    Quote Originally Posted by Coder Dave View Post
    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.

  6. #21
    Join Date
    Feb 2013
    Posts
    10

    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
    Last edited by Kegs; March 2nd, 2013 at 07:32 AM.

  7. #22
    Join Date
    Dec 2012
    Location
    England
    Posts
    2,460

    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.

  8. #23
    Join Date
    Jun 2010
    Location
    Germany
    Posts
    2,587

    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.
    Last edited by Eri523; March 2nd, 2013 at 08:01 AM.
    I was thrown out of college for cheating on the metaphysics exam; I looked into the soul of the boy sitting next to me.

    This is a snakeskin jacket! And for me it's a symbol of my individuality, and my belief... in personal freedom.

  9. #24
    Join Date
    Dec 2012
    Location
    England
    Posts
    2,460

    Re: [Advice Needed] Noob Code!

    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.

  10. #25
    Join Date
    Feb 2013
    Posts
    10

    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.

  11. #26
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Posts
    12,108

    Re: [Advice Needed] Noob Code!

    Quote Originally Posted by Kegs View Post
    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.
    Last edited by GCDEF; March 2nd, 2013 at 08:21 AM.

  12. #27
    Join Date
    Dec 2012
    Location
    England
    Posts
    2,460

    Re: [Advice Needed] Noob Code!

    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.

  13. #28
    Join Date
    Feb 2013
    Posts
    10

    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.

Page 2 of 2 FirstFirst 12

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This is a CodeGuru survey question.


Featured


HTML5 Development Center