1. ## 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. 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.

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

5. 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.

6. 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.

7. Originally Posted by manasij7479
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

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. Originally Posted by Cowmoogun
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
}```

9. 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. Originally Posted by anduril462
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. You have an error in Heron's formula. Side1 is in there twice, side2, not at all.