Like Tree1Likes

Grade my first lab? (and program :D)

This is a discussion on Grade my first lab? (and program :D) within the C Programming forums, part of the General Programming Boards category; Well this is my first program and it runs well with no bugs =D, but that doesnt do much if ...

  1. #1
    Registered User
    Join Date
    Feb 2012
    Location
    Winnetka, California, United States
    Posts
    15

    Grade my first lab? (and program :D)

    Well this is my first program and it runs well with no bugs =D, but that doesnt do much if the professor doesnt think the code is modular or easy to read so I was wondering if you could give some feedback on this program. It's very simple, user inputs three numbers and it will respond with what type of triangle it is and its area with a loop. I appreciate any and all feedback

    Code:
    #include <stdio.h>
    #include <conio.h>
    #include <math.h>
    
    
    
    
    int main ()
    {
    	int side1, side2, side3;
    	double S; 
    	double Area;
    	
    
    
    	do{ printf("\nPlease enter three sides of a triangle.\n");	/* S is the semiperimeter of a triangle which is essential in finding*/
    		scanf("%d%d%d", &side1, &side2, &side3);				/*	a consistent area for the program.								 */
    		S= (float)(side1+side2+side3)/2;
    		Area = sqrt (S * (S-side1) *(S-side2)*(S-side3));
    					
    			if (((side1 == side2) && (side2 == side3) && (side1 == side3)) && ((side1 + side2 > side3) && (side2 + side3 > side1) && (side1 + side3 > side2)))	/* All sides must be equal */
    																																								/* and follow the general laws of a triangle:*/
    				{																																				/* The sum of any two sides must be greater than the remaining side. */
    				printf("This triangle's sides are %d,%d,%d and is an Equalateral triangle.\nIts area is %.05f.", side1, side2, side3, Area);
    			
    			}
    			else if (((side1 == side2) || (side2 == side3) || (side1 == side3)) && (side1 + side2 >side3) &&  (side2 + side3 >side1) &&  (side1 + side3 > side2))  /* Two sides must be equal */
    				{
    				printf("This triangle's sides are %d,%d,%d and is an Isosceles triangle.\nIts area is %.05f.", side1, side2, side3, Area);
    
    
    			}
    
    
    			
    			else if ((side1 != side2) && (side3 != side2)&& (side1 != side3) && (side1 + side2 >side3 && side2 + side3 >side1 &&  side1 + side3 > side2))			/* None of the sides can be the same */
    				{
    				printf("This triangle's sides are %d,%d,%d and is an Scalene triangle.\n Its area is %.05f.", side1, side2, side3, Area);
    			
    			}
    			
    			else 
    				{	
    				printf("%d,%d,%d are not valid sides to a triangle.\n", side1,side2,side3);	/* Will end the program once user inputs an incorrect combination of numbers. */
    				getch();
    			}
    			
    	  }while (side1 + side2 >side3 && side2 + side3 >side1 && side1 + side3 > side2);
    
    
    		return 0;
    		getch();
    }

  2. #2
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    I did not look through the entire program, but this struck me:
    Code:
    if (((side1 == side2) && (side2 == side3) && (side1 == side3))
    If side1 == side2 && side2 == side3, then you've already proven than side1 == side3, so no need to test that again.

  3. #3
    Registered User
    Join Date
    Feb 2012
    Location
    Winnetka, California, United States
    Posts
    15
    Quote Originally Posted by Adak View Post
    I did not look through the entire program, but this struck me:
    Code:
    if (((side1 == side2) && (side2 == side3) && (side1 == side3))
    If side1 == side2 && side2 == side3, then you've already proven than side1 == side3, so no need to test that again.
    Ah yes that is a bit redundant, thank you for pointing that out!

  4. #4
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,523
    You can structure it better (as some of the conditions are being checked many times).
    Code:
     if(<check if triangle is valid>)
    {
        if(<check for equilateral>)
            //...
        else if (<check for isosceles>)
            //...
        else
            //scalene
    }
    else
       //<print your error>
    Last edited by manasij7479; 02-21-2012 at 02:49 AM.
    Manasij Mukherjee | gcc-4.9.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  5. #5
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,139
    Some formal feedback. Note that it's style and can (and will) be argued.

    Remove any empty lines after the first in each empty space. One empty line is enough to space things out.

    Your side variables are lowercase, your S variable is uppercase and Area is so called Pascal-Case. Decide which you want (all lowercase is common for local variables) and stick with it for all three/five.

    Each curly brace should reside on a line of it's own. From each closing brace, the corresponding opening brace should be straight up so you just have to draw a vertical line to find the two matching braces. Look at manasij7479s code to get a feeling for it.
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

  6. #6
    Registered User
    Join Date
    May 2009
    Posts
    2,765
    Style issue on this line.
    Code:
    S= (float)(side1+side2+side3)/2;
    I would used a better name for "S" as others has stated.

    I would tend to use this code below. I avoid cast if possible.
    Code:
    S= (side1+side2+side3)/2.0;
    or you might use this code (S is of type double)
    Code:
    S= (double)(side1+side2+side3)/2;
    Tim S.
    "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the universe trying to produce bigger and better idiots. So far, the Universe is winning." Rick Cook

  7. #7
    Registered User
    Join Date
    Feb 2012
    Location
    Winnetka, California, United States
    Posts
    15
    Quote Originally Posted by manasij7479 View Post
    You can structure it better (as some of the conditions are being checked many times).
    Code:
     if(<check if triangle is valid>)
    {
        if(<check for equilateral>)
            //...
        else if (<check for isosceles>)
            //...
        else
            //scalene
    }
    else
       //<print your error>

    I appreciate all the comments so far and I'm implementing them to stream line the code but i'm coming across a new problem in my do while statement. Now in my while statement i have the law of a triangle, so i took them out of the if statements and now it will say it's a triangle even when it isn't -__-. Anyone know why this is? Wouldnt it check to make sure it fits within the while statement so it doesnt "do it."

  8. #8
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,523
    Quote Originally Posted by Cowmoogun View Post
    I appreciate all the comments so far and I'm implementing them to stream line the code but i'm coming across a new problem in my do while statement. Now in my while statement i have the law of a triangle, so i took them out of the if statements and now it will say it's a triangle even when it isn't -__-. Anyone know why this is? Wouldnt it check to make sure it fits within the while statement so it doesnt "do it."
    That is because you used a do while loop.
    For the first time, it gets executed regardless of the conditions.
    Change
    Code:
    do {...}while (condition);
    to
    Code:
    while(condition){...}
    But then, the values will be checked before they are entered.
    A better solution is to:
    Code:
    while(1)//infinite loop
    {
        do
        {
             //User input
        }while(<triangle is NOT valid>);//
        //Everything else
    }
    Manasij Mukherjee | gcc-4.9.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  9. #9
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,676
    Please post your new code. Also, you could "flatten" the structure manasij7479 suggested:
    Code:
    if triangle is invalid
        print error
    else if equilateral
        ...
    else if isosceles
        ...
    else
        // scalene
    Your first call to getch should probably not be inside the else clause, otherwise it wont always be executed (i.e. why would you only want to call getch when they entered an invalid input). Your last call to getch is after the return 0; in main, so it will never get executed. Either remove it or switch the two lines.

    Lastly, as others pointed out, S is poorly names. So much so that you need a comment to explain that S is the semiperimeter. Just call it semiperimeter and be done with it.

  10. #10
    Registered User
    Join Date
    Feb 2012
    Location
    Winnetka, California, United States
    Posts
    15
    Quote Originally Posted by anduril462 View Post
    Please post your new code. Also, you could "flatten" the structure manasij7479 suggested:
    Code:
    if triangle is invalid
        print error
    else if equilateral
        ...
    else if isosceles
        ...
    else
        // scalene
    Your first call to getch should probably not be inside the else clause, otherwise it wont always be executed (i.e. why would you only want to call getch when they entered an invalid input). Your last call to getch is after the return 0; in main, so it will never get executed. Either remove it or switch the two lines.

    Lastly, as others pointed out, S is poorly names. So much so that you need a comment to explain that S is the semiperimeter. Just call it semiperimeter and be done with it.
    Here is the new code I have, I rewrote it..but now the weirdest thing..the math for my areas is wrong. For example..sides 3,4,5 should have area 6 but now im getting 7.2 -_-.

    Code:
    #include <stdio.h>
    #include <conio.h>
    #include <math.h>
    
    
    int main ()
    {
        int side1, side2, side3;                
        double semiperimeter, area;                                                    
    
    
                                                                        
            do
            {      printf("\n Please enter three sides to a triangle. \n");
               scanf("%d%d%d", &side1, &side2, &side3);
                semiperimeter = (double)(side1 + side2 + side3)/2;
               area = sqrt(semiperimeter * (semiperimeter - side1) * (semiperimeter - side1) * (semiperimeter - side3));
              
    
    
               if (side1 + side2 > side3 && side2 + side3 > side1 && side1 + side3 > side2)
               {    if ((side1 == side2) && (side2 == side3))
                        printf("This triangles sides are %d,%d,%d and is an equalateral triangles.\nIts area is %.05f.\n", side1, side2, side3, area);
                    
                    else if ((side1 == side2) || (side2 == side3) || (side1 == side3))
                        printf("This triangles sides are %d,%d,%d and is an Isosceles triangle.\nIts area is %.05f.\n", side1, side2, side2, area);
                    
                    else 
                       printf("This triangles sides are %d,%d,%d and is a Scalene triangle.\nIts area is %.05f.\n", side1, side2, side3, area);
               }
               else
                   printf("%d,%d,%d are not valid sides to a triangle.\n", side1,side2,side3);
            }while ((side1 + side2 >side3) && (side2 + side3 >side1) && (side1 + side3 > side2));
        
            getch();
            return 0;
    }

  11. #11
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    You have an error in Heron's formula. Side1 is in there twice, side2, not at all.

  12. #12
    Registered User
    Join Date
    Feb 2012
    Location
    Winnetka, California, United States
    Posts
    15
    Quote Originally Posted by Adak View Post
    You have an error in Heron's formula. Side1 is in there twice, side2, not at all.
    Wow...I want to kill myself with that mistake. Really have to work on my attention to detail, thank you. How is the layout of the code now based on your opinion? Is it cleaner then the first?

  13. #13
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,676
    Much better, though the indentation on line 16 seems to be off by one, and everything from lines 13-36 should be de-indented one step, so it lines up with the declarations on lines 8-9. As an issue of personal preference, I would move the first statment of a block onto a line of it's own (i.e. don't share it with the opening curly bracket). Also, I would remove (subjectively) unnecessary blank lines (10, 11, 18, 19, 24, 27). My suggestion is to pick one of the first 3 styles mentioned on Indent style - Wikipedia, the free encyclopedia, and stick to it.

  14. #14
    Registered User
    Join Date
    Feb 2012
    Location
    Winnetka, California, United States
    Posts
    15
    Quote Originally Posted by anduril462 View Post
    Much better, though the indentation on line 16 seems to be off by one, and everything from lines 13-36 should be de-indented one step, so it lines up with the declarations on lines 8-9. As an issue of personal preference, I would move the first statment of a block onto a line of it's own (i.e. don't share it with the opening curly bracket). Also, I would remove (subjectively) unnecessary blank lines (10, 11, 18, 19, 24, 27). My suggestion is to pick one of the first 3 styles mentioned on Indent style - Wikipedia, the free encyclopedia, and stick to it.
    I really appreciate the responses on this forum, I've received a lot of help from all who responded! Anduril, i'm doing exactly what you told me to do now, i'll post the final code when I'm completed and i'll let you guys know what my professor gave me afterwards .

  15. #15
    Registered User rogster001's Avatar
    Join Date
    Aug 2006
    Location
    Liverpool UK
    Posts
    1,438
    You have certainly taken on board the advice given, the code is more concise and readable, it is good that you dropped the 'S' variable, apart from your general changes to remove this, THINGS_IN_CAPS should be reserved for constant values.
    A further point is conio.h is not standard, you should not rely on this in your code, and consider your compiler choice, if you are using turbo c IDE change it.
    Last edited by rogster001; 02-21-2012 at 03:52 PM.
    Thought for the day:
    "Are you sure your sanity chip is fully screwed in sir?" (Kryten)
    FLTK: "The most fun you can have with your clothes on."

    Stroustrup:
    "If I had thought of it and had some marketing sense every computer and just about any gadget would have had a little 'C++ Inside' sticker on it'"

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Please Help Grade Curving program!!!
    By phil12 in forum C++ Programming
    Replies: 5
    Last Post: 04-07-2011, 10:23 PM
  2. Grade Program
    By Jbrubin in forum C Programming
    Replies: 2
    Last Post: 03-28-2011, 03:02 PM
  3. Replies: 2
    Last Post: 01-29-2011, 12:58 PM
  4. Student Grade Program
    By Vinod Menon in forum C Programming
    Replies: 1
    Last Post: 05-31-2004, 01:32 AM
  5. Student Grade Program
    By Vinod Menon in forum C Programming
    Replies: 5
    Last Post: 05-28-2004, 03:49 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21