7
\$\begingroup\$

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");
                }
        }
}

\$\endgroup\$
  • 1
    \$\begingroup\$ Why are there so many Sleep statements in your code? \$\endgroup\$ – Aniruddha Deb May 30 at 14:31
  • \$\begingroup\$ @AniruddhaDeb Oh, cause need to load, like i am trying not to display the output instantly, but with time gap, so it looks more appropriate. What are you views? \$\endgroup\$ – UnfreeHeX May 31 at 2:10
  • 4
    \$\begingroup\$ I don't think you should do that in this case. It would be embarrassing if a pocket calculator finds the answer quicker than yours :) \$\endgroup\$ – Aniruddha Deb May 31 at 3:10
  • 3
    \$\begingroup\$ You only display loading animations to give the user something to look at if you actually take a long time to load... If your app can load instantly, then there's no point in wasting time to display animations. \$\endgroup\$ – Cody Gray May 31 at 10:53
  • \$\begingroup\$ @UnfreeHeX It doesn't look more appropriate. It looks just like any other braindead bloatware that wastes people's time to do the simplest of things. Do not emulate stupidity. It doesn't make it look good. Those "loading" screens are horrible, they are an usually an indicator that the software is crap. Please, don't propagate this. Be happy you can have something that can be easily made fast. \$\endgroup\$ – Reinstate Monica Jun 11 at 4:08
10
\$\begingroup\$

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 expression x * PI/180. This looks a lot like "converting x 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."

| improve this answer | |
\$\endgroup\$
  • 3
    \$\begingroup\$ The constant macro M_PI in math.h is not in the C-standard, although it is in Posix (pubs.opengroup.org/onlinepubs/009695399/basedefs/math.h.html). \$\endgroup\$ – deamentiaemundi May 30 at 15:33
  • 3
    \$\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 after printf. \$\endgroup\$ – lights0123 May 30 at 17:01
  • \$\begingroup\$ Extra );: fixed! Thanks. Re the self-explanatoriness of 3.14 in a function named to_radians: I suspect we agree more than we disagree. :) \$\endgroup\$ – Quuxplusone May 30 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 an int. godbolt.org/z/cCyhfa \$\endgroup\$ – Bob__ May 30 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 Jun 1 at 23:30
3
\$\begingroup\$

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.

| improve this answer | |
\$\endgroup\$
  • 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\$ – Mast May 30 at 13:40
  • \$\begingroup\$ Thanks for the Welcome :). \$\endgroup\$ – mangupt May 30 at 13:43
1
\$\begingroup\$

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);
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I suppose the word sind is a typo in your answer for sin ? 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 Jun 14 at 15:45
  • \$\begingroup\$ @Pryftan sind(angle) implies angle is in degrees, not radians. \$\endgroup\$ – chux - Reinstate Monica Jun 15 at 0:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.