This code below, was for my school project, first year. I am new to C programming, before which I learned Python. Hence, I do not know the tweaks and tricks in C language. What are the various ways to improve the code? Moreover, my requirement requires me to have indentation. I am unsure how to apply that indentation in my code. My code needs to be user-friendly and must have smooth execution ( nice to view ).
#include <stdio.h>
#include <stdlib.h> //For functions like system() and exit()
#include <windows.h> //For function Sleep()
#include <math.h> //For functions like pow(), sin(), cos(), tan()
#define PI 3.141592654 //Function is being referred at first so as to use it in main.
int main(void)
{
int i = 1; /* */
int Reuse; /* */
double x, xy, y; /* */
char Opt; /* Declaring the type variables */
int Numbering; /* */
int N, F; /* */
float Num1, Num2 ,ans; /* */
char oper; /* */
printf("Welcome to our calculator.\n");
while (1){ //While loop that never ends, unless exit(0) is used
printf("\n\nWhich mode do you want to use?\n1.Normal maths operations\n2.Trigonometric functions\n3.Fibonacci Series\n4.Exit\n\nYour input: ");
scanf(" %c", &Opt);
if (Opt == '1'){
printf("Welcome to Normal maths operation Mode.\n\nYour two numbers: ");
scanf("%f%f", &Num1, &Num2);
printf("\nAVAILABLE SYMBOLS:\n\n+ for Addition\n- for Subtraction\n/ for Division\n* for Multiplication\n^ for Power function\n\nYour input: ");
scanf(" %c", &oper);
if (oper == '+'){
ans = (Num1 + Num2);
printf("Here is your answer:\n%f %c %f = %.5f (To 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '-'){
ans = (Num1 - Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '/'){
ans = (Num1 / Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '*'){
ans = (Num1 * Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '^'){
ans = (pow (Num1 , Num2));
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else{
printf("\n\nYour input operator is incorrect; ERROR 1 Sintek\n");
Sleep(2450);
system("cls");
}
}
if (Opt == '2'){
printf("Welcome to Trigonometric Function Mode.\n\nInput your angle in degrees: ");
scanf("%f", &Num1);
printf("The trigo you are going to use\ns for sine\nc for cosine\nt for tangent\nYour input: ");
scanf(" %c", &oper);
if (oper == 's'){
ans = (sin (Num1 * PI/180));
printf("\nHere is your answer:\nAngle: %f\nSin%f = %f", Num1, Num1, ans);
Sleep(2450);
} else if (oper == 'c'){
ans = (cos (Num1 * PI/180));
printf("\nHere is your answer:\nAngle: %f\nCos%f = %f", Num1, Num1, ans);
Sleep(2450);
} else if (oper == 't'){
ans = (tan (Num1 * PI/180));
printf("\nHere is your answer:\nAngle: %f\nTan%f = %f", Num1, Num1, ans);
Sleep(2450);
} else{
printf("\n\nWrong operator used for Trigo; ERROR 1 Sintek");
Sleep(2450);
system("cls");
}
}
if (Opt == '3'){
printf("Welcome to Fibonacci Series Mode.\n\nEnter how many numbers do you want from the series, from the start: ");
scanf("%d", &N);
x = 0;
y = 1;
F = 3;
Numbering = 3;
printf("Here is Your Series:\n\n");
if (N == 1){
printf("[1] 0\n");
Sleep(1000);
}
if (N == 2){
printf("[1] 0\n");
Sleep(250);
printf("[2] 1\n");
Sleep(1000);
}
if (N == 3){
printf("[1] 0\n");
Sleep(250);
printf("[2] 1\n");
Sleep(250);
printf("[3] 1\n");
Sleep(250);
}
if (N > 3){
printf("[1] 0\n");
Sleep(250);
printf("[2] 1\n");
Sleep(250);
}
while ( N > 3 && F <= N ){
xy = x + y;
printf("[%.0d] %.0lf\n", Numbering, xy);
Sleep(250);
x = y;
y = xy;
F++;
Numbering++;
}
Sleep(1000);
}
if (Opt == '4'){
printf("Thank you for using my calculator. Hope to see you again!!");
Sleep(1990);
system("cls");
exit(0);
}
if (Opt != '1' && Opt!= '2' && Opt!= '3' && Opt != '4'){
printf("Wrong Option. Please retype your option correctly");
Sleep(2450);
system("cls");
}
}
}
3 Answers 3
My requirement requires me to have indentation. I am unsure how to apply that indentation in my code.
Just indent your C code exactly the same way you'd indent Python code. Start at the left margin (column 0), and then each time you "go in a level" (in the body of a function, or an if
or while
or for
, or when breaking an expression across multiple lines), just space over by 4. For example, you wrote this before:
if (Opt == '1'){
printf("Welcome to Normal maths operation Mode.\n\nYour two numbers: ");
scanf("%f%f", &Num1, &Num2);
printf("\nAVAILABLE SYMBOLS:\n\n+ for Addition\n- for Subtraction\n/ for Division\n* for Multiplication\n^ for Power function\n\nYour input: ");
scanf(" %c", &oper);
if (oper == '+'){
ans = (Num1 + Num2);
printf("Here is your answer:\n%f %c %f = %.5f (To 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '-'){
ans = (Num1 - Num2);
Instead, just think "What would Python do?" and then do that.
if (Opt == '1') {
printf("Welcome to Normal maths operation Mode.\n\n");
printf("Your two numbers: ");
scanf("%f%f", &Num1, &Num2);
printf(
"\nAVAILABLE SYMBOLS:\n\n"
"+ for Addition\n"
"- for Subtraction\n"
"/ for Division\n"
"* for Multiplication\n"
"^ for Power function\n\n"
);
printf("Your input: ");
scanf(" %c", &oper);
if (oper == '+') {
ans = Num1 + Num2;
printf("Here is your answer:\n");
printf(
"%f %c %f = %.5f (To 5 decimal places)\n\n",
Num1, oper, Num2, ans
);
Sleep(2450);
} else if (oper == '-') {
ans = Num1 - Num2;
Another good solution is to run clang-format
over your source file, or use a text editor that understands curly braces and can indent for you.
#define PI 3.141592654
load(); //Function is being referred at first so as to use it in main.
int main(void)
Whoa — there's a comment on that line! I didn't even see it in your question, because you'd put it insanely far over to the right. Don't do that. You want people to see these comments; that's why you wrote them, right? So indent them just as you would in Python.
Secondly: load();
is a function call expression (or in this case, an expression statement). You can't have a function call just dangling out at file scope. Every statement must go inside some function (e.g. main
).
But, I can tell from context that what you mean was to forward-declare the function load
. The way you write a function declaration in C is, exactly the same as a function definition — except you omit the body! So, to forward-declare
void load() {
...
}
you would write
//Function is being referred at first so as to use it in main.
void load();
(The comment is pretty pointless, actually. I just included it to show how you should indent comments, i.e., nothing special.)
Finally, that #define
for PI
:
The C standard library already defines
M_PI
in<math.h>
. So you could have just used that.You only ever use
PI
as part of the expressionx * PI/180
. This looks a lot like "convertingx
to radians." That's a named operation in English; it should be a named function in your C program.#define PI 3.141592654 double to_radians(double degrees) { return degrees * PI / 180; }
Now you have only a single use of PI
in your whole program, and so you don't save anything by giving it a name. Eliminate the macro:
double to_radians(double degrees) {
return degrees * (3.141592654 / 180.0);
}
I've also parenthesized the constant part in hopes that the constant-folder will do the arithmetic ahead of time. That might be unnecessary, but it certainly can't hurt anything.
In general, your main
function is much much too long. Figure out some logical way to split it up into functions. For example, you might say
if (Opt == '1') {
do_normal_maths_mode();
} else if (Opt == '2') {
do_trigonometric_function_mode();
} else if (Opt == '3') {
do_fibonacci_series_mode();
} else if (Opt == '4') {
print_greeting_and_exit();
} else {
printf("Wrong Option. Please retype your option correctly\n");
Sleep(2450);
system("cls");
}
Notice that I'm using a terminal else
clause on my if
— just like I would in Python! (although Python uses elif
instead of else if
) — so that any Opt
other than 1, 2, 3, or 4 will drop into the else
branch and print "Wrong Option." You don't have to test (Opt != '1' && Opt!= '2' && Opt!= '3' && Opt != '4')
manually.
C does provide a control-flow structure that Python doesn't: the switch. It would look like this:
switch (Opt) {
case '1':
do_normal_maths_mode();
break;
case '2':
do_trigonometric_function_mode();
break;
case '3':
do_fibonacci_series_mode();
break;
case '4':
print_greeting_and_exit();
break;
default:
printf("Wrong Option. Please retype your option correctly\n");
Sleep(2450);
system("cls");
break;
}
However, I wouldn't really recommend a switch
in this case, because it's more lines of code and easier to mess up. (For example, you might accidentally forget one of those break
statements.) Any mainstream compiler will generate equally efficient code for either version: the if-else
chain or the switch
statement.
There's more that can be said, but I'll stop here. The big giant enormous issue is "you need to break your code up into functions."
-
3\$\begingroup\$ The constant macro
M_PI
inmath.h
is not in the C-standard, although it is in Posix (pubs.opengroup.org/onlinepubs/009695399/basedefs/math.h.html). \$\endgroup\$deamentiaemundi– deamentiaemundi2020年05月30日 15:33:18 +00:00Commented May 30, 2020 at 15:33 -
4\$\begingroup\$ "Now you have only a single use of
PI
in your whole program, and so you don't save anything by giving it a name. Eliminate the macro" It's fairly obvious in this case, but now you have a magic number that doesn't explain what the value is. There's also an extra);
in your second code block afterprintf
. \$\endgroup\$lights0123– lights01232020年05月30日 17:01:13 +00:00Commented May 30, 2020 at 17:01 -
\$\begingroup\$ Extra
);
: fixed! Thanks. Re the self-explanatoriness of3.14
in a function namedto_radians
: I suspect we agree more than we disagree. :) \$\endgroup\$Quuxplusone– Quuxplusone2020年05月30日 17:05:37 +00:00Commented May 30, 2020 at 17:05 -
2\$\begingroup\$ Please note that
load();
, in that position, is actually a declaration of a function that takes an unspecified number of parameters of unknown type implicitly returning anint
. godbolt.org/z/cCyhfa \$\endgroup\$Bob__– Bob__2020年05月30日 20:17:13 +00:00Commented May 30, 2020 at 20:17 -
2\$\begingroup\$ You use
system("cls")
a lot. It would be more efficient to use ANSI escape codes IF your terminal emulator supports them. From memory (not sure tho) newer Windows Console versions support them, as does Windows Terminal. The main advantage to this approach is the lack of an external process. Example Usage:printf("\x1B[2J\x1B[H");
. \$\endgroup\$user692992– user6929922020年06月01日 23:30:23 +00:00Commented Jun 1, 2020 at 23:30
Please use a small function for print, sleep and clear screen:
void output(const char* msg, int sleep_time, int clear) {
printf("%s", msg);
sleep(sleep_time);
if (clear) system("cls");
}
Order of these functions can be you choice and you can control the sleep time using parameter.
-
1\$\begingroup\$ Welcome to Code Review! It looks like your last sentence is incomplete. Did you perhaps mean to write a more complete answer? \$\endgroup\$2020年05月30日 13:40:23 +00:00Commented May 30, 2020 at 13:40
-
\$\begingroup\$ Thanks for the Welcome :). \$\endgroup\$mangupt– mangupt2020年05月30日 13:43:04 +00:00Commented May 30, 2020 at 13:43
What are the various ways to improve the code(?)
PI
Why code a coarse machine pi as used in double
math (good to 15+ decimal places) when a better value is a copy and paste away?
Some systems provide M_PI
. That is non-standard.
#ifdef M_PI
#define PI M_PI
#else
// #define PI 3.141592654
#define PI 3.1415926535897932384626433832795
#endif
Old style declaration
load();
does not declare the return type nor the parameters.
// load();
void load(void);
FP precision
"%.5f"
makes small answers all "0.00000" and large values verbose 123456789012345.00000. Recommend instead %.5g
which shifts to exponential notation for large and small values.
Code re-use
Below code repeated many times. Use a helper function.
void print_results(double NUm1, int oper, double Num2, double ans) {
printf("Here is your answer:\n%f %c %f = %.5f (To 5 decimal places)\n\n",
Num1, oper, Num2, ans);
Sleep(2450);
}
Samples calls
...
} else if (oper == '-'){
print_results(Num1, oper, Num2, Num1 - Num2);
} else if (oper == '/'){
print_results(Num1, oper, Num2, Num1 / Num2);
}
...
Advanced: sind(deg)
for large deg
When code is attempting to do trig on large degree values, rather than scale by PI/180
and then call sin(), cos(), ...
, perform argument reduction in degrees as that can be done exactly - then scale. You will get better answers for large degree values. Sin and Cos give unexpected results for well-known angles. Of course when only printing a few digits, you may not see this improvement often, yet it is there.
// ans = (sin (Num1 * PI/180));
ans = fmod(Num1, 360);
ans = sin(Num1 * PI/180);
Simplify
With digits, a range test can be used
// if (Opt != '1' && Opt!= '2' && Opt!= '3' && Opt != '4'){
if (Opt < '1' || Opt > '4') {
My requirement requires me to have indentation. I am unsure how to apply that indentation in my code.
Life is too short to manually indent. Use (or find) your IDE's code formatter and use that.
// OP's
if (oper == '+'){
ans = (Num1 + Num2);
printf("Here is your answer:\n%f %c %f = %.5f (To 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '-'){
ans = (Num1 - Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '/'){
ans = (Num1 / Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
vs.
// Eclipse
if (oper == '+') {
ans = (Num1 + Num2);
printf("Here is your answer:\n%f %c %f = %.5f (To 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '-') {
ans = (Num1 - Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
} else if (oper == '/') {
ans = (Num1 / Num2);
printf("Here is your answer:\n%f %c %f = %.5f (to 5 decimal places)\n\n", Num1, oper, Num2, ans);
Sleep(2450);
-
\$\begingroup\$ I suppose the word
sind
is a typo in your answer forsin
? Oh and have a +1 for the well rounded answer - and for pointing out the range check (I was going to make an answer or comment about that but saw you already had that done). \$\endgroup\$Pryftan– Pryftan2020年06月14日 15:45:11 +00:00Commented Jun 14, 2020 at 15:45 -
\$\begingroup\$ @Pryftan
sind(angle)
impliesangle
is in degrees, not radians. \$\endgroup\$chux– chux2020年06月15日 00:11:55 +00:00Commented Jun 15, 2020 at 0:11
Sleep
statements in your code? \$\endgroup\$