15
\$\begingroup\$

I am new to C programming, I previously studied Python. Below is my year one project in C.

My project's name is "Advanced Calculator", it's a menu-driven calculator application with several operations, as you can see from the first menu:

Which mode do you want to use?
[1] Normal maths operations
[2] Special Functions
[3] Fibonacci Series
[4] Random Mathematical Question
[5] Exit

Your input:

Although the calculator works so far and all operations are completed, I feel that the program lacks some features and functions that are typically found in any calculator. I plan to add sinh, tanh and cosh into it, but what else? Any idea is appreciated!

#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()
#include <time.h>          //For time based modules and functions
#include <conio.h>         //For kbhit, input detector
#define PI 3.14159265358979323846
load();                    //Function Prototype
main(void)
{
    int i = 1;                         /*                                    */
    double x, xy, y;                   /*                                    */
    char Opt;                          /*   Declaring the type variables     */
    int Numbering;                     /*                                    */
    int N, F, Oof, Check;              /*                                    */
    int a, b, Choice;                  /*                                    */
    int c, d, K;                       /*                                    */
    float Num1, Num2 ,ans, CheckF;     /*                                    */
    char oper, H;                      /*                                    */
    system("cls");         //Clears terminal screen
    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?\n[1] Normal maths operations\n[2] Special Functions\n[3] Fibonacci Series\n[4] Random Mathematical Question\n[5] 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\\ for Rooting\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(245);
                } 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(245);
                } 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(245);
                } 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(245);
                } 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(245);
                } else if (oper == '\\'){
                    ans   = pow(Num2 , 1/Num1);
                    Check = Num1;
                    Oof   = Check % 2;
                    if (Num2 < 0){
                        printf("Cannot root a negative number; ERROR 1 Sintek\a\n");
                        system("pause");
                        system("cls");
                        } else if (Oof == 0){
                            printf("Here is your answer:\n%f root(%f) = - %.5f or + %.5f (to 5 decimal places)\n\n", Num1, Num2, ans, ans);
                        Sleep(245);
                        } else if (!Oof == 0){
                            printf("Here is your answer:\n%f root(%f) = + %.5f (to 5 decimal places)\n\n", Num1, Num2, ans);
                            Sleep(245);
                        }
                    } else {
                    printf("\n\nYour input operator is incorrect; ERROR 1 Sintek\n");
                    printf("\a\n");
                    system("pause");
                    system("cls");
                }
                }
            if (Opt == '2'){
            printf("Welcome to Special Functions Mode.\n\n[1] Sine Function\n[2] Cosine Function\n[3] Tangent Function\n[4] Log (With base 10)\n[5] Log (With base e)\n[6] Log (With user defined base)\n[7] Sine Inverse Function\n[8] Cosine Inverse Function\n[9] Tangent Inverse Function\n\nWhich mode do you want: ");
            scanf("%d", &N);
            if (N == 1){
                printf("Your angle: ");
                scanf("%f", &Num1);
                ans = (sin ( Num1 * PI/180 ));
                printf("\nHere is your answer:\nSine(%f) = %.5f (to 5 decimal places)\n\n", Num1, ans);
                Sleep(245);
                } else if (N == 2){
                    printf("Your angle: ");
                    scanf("%f", &Num1);
                    ans = (cos ( Num1 * PI/180 ));
                    printf("Here is your answer:\nCosine(%f) = %.5f (to 5 decimal places)\n\n", Num1, ans);
                    Sleep(245);
                    } else if (N == 3){
                        printf("Your angle: ");
                        scanf("%f", &Num1);
                        ans = (tan ( Num1 * PI/180 ));
                        printf("Here is your answer:\nTangent(%f) = %.5f (to 5 decimal places)\n\n", Num1, ans);
                        Sleep(245);
                        } else if (N == 4){
                            printf("Your number: ");
                            scanf("%f", &Num1);
                            ans = log10(Num1);
                            if (Num1 < 0){
                                printf("Cannot log a negative number; ERROR 1 Sintek\a\n");
                                system("pause");
                                system("cls");
                                } else if (Num1 == 0){
                                    printf("Cannot log(0); ERROR 1 Sintek\a\n");
                                    system("pause");
                                    system("cls");
                                    } else if (Num1 > 0){
                                    printf("Here is your answer:\nLg(%f) = %.5f (to 5 decimal places)\n\n", Num1, ans);
                                    Sleep(245);
                                }
                            } else if (N == 5){
                                printf("Your number: ");
                                scanf("%f", &Num1);
                                ans = log(Num1);
                                if (Num1 < 0){
                                    printf("Cannot ln a negative number; ERROR 1 Sintek\n\a");
                                    system("pause");
                                    system("cls");
                                    } else if (Num1 == 0){
                                        printf("Cannot ln(0); Error 1 Sintek\n\a");
                                        system("pause");
                                        system("cls");
                                        } else if (Num1 > 0){
                                            printf("Here is your answer:\nLn(%f) = %.5f (to 5 decimal places)\n\n", Num1, ans);
                                            Sleep(245);
                                        }
                                } else if (N == 6){
                                    printf("Enter the base number, followed by the number: ");
                                    scanf("%f%f", &Num1, &Num2);
                                    ans = ( log(Num2) / log(Num1));
                                    if (Num1 <= 0 || Num2 <=0){
                                        printf("Cannot log a negative/zero base/number; ERROR 1 Sintek\n\a");
                                        system("pause");
                                        system("cls");
                                        } else if (Num1 > 0 && Num2 > 0){
                                            printf("Here is your answer:\nLog[base %f]%f = %.5f (to 5 decimal places)\n\n", Num1, Num2, ans);
                                            Sleep(245);
                                        }
                                 } else if (N == 7){
                                     printf("[1] Entering hypotenuse and opposite of triangle\n[2] Entering the value directly\n\nYour option: ");
                                     scanf("%d", &K);
                                     if (K == 1){
                                         printf("Enter hypotenuse and opposite sides of the triangle: ");
                                         scanf("%f%f", &Num1, &Num2);
                                         CheckF = Num2 / Num1;
                                         if (CheckF < -1 || CheckF > 1){
                                             printf("The opposite side should not be larger than the hypotenuse side. Please recheck your values!\nERROR 1 Sintek\n\a");
                                             system("pause");
                                             system("cls");
                                             } else {
                                                 ans = (asin ( CheckF ));
                                                 printf("Sine inverse %f/%f =\n%f (In radians)", Num2, Num1, ans);
                                                 ans = ans * 180/PI;
                                                 printf("\n%f (In degrees)", ans);
                                                 Sleep(250);
                                             }
                                             } else if (K == 2){
                                                 printf("Enter your value: ");
                                                 scanf("%f", &CheckF);
                                                 if (CheckF < -1 || CheckF > 1){
                                                     printf("Value cannot be higher than 1/lower than -1. Please recheck your input!\nERROR 1 Sintek\n\a");
                                                     system("pause");
                                                     system("cls");
                                                     } else {
                                                         ans = (asin ( CheckF ));
                                                         printf("Sine inverse %f =\n%f (In radians)", CheckF, ans);
                                                         ans = ans * 180/PI;
                                                         printf("\n%f (In degrees)", ans);
                                                         Sleep(250);
                                                     }
                                                     } else if (K != 1 || K != 2) {
                                                         printf("Your input option is not found! ERROR 404\a\n");
                                                         system("pause");
                                                         system("cls");
                                                     }
                                   } else if (N == 8){
                                       printf("[1] Entering adjacent and hypotenuse of triangle\n[2] Entering the value directly\n\nYour option: ");
                                       scanf("%d", &K);
                                       if (K == 1){
                                           printf("Enter adjacent and hypotenuse sides of the triangle: ");
                                           scanf("%f%f", &Num1, &Num2);
                                           CheckF = Num1 / Num2;
                                           if (CheckF < -1 || CheckF > 1){
                                               printf("The adjacent side should not be larger than the hypotenuse side. Please reckeck your values!\nERROR 1 Sintek\n\a");
                                               system("pause");
                                               system("cls");
                                               } else {
                                                   ans = (acos ( CheckF ));
                                                   printf("Cosine inverse %f/%f =\n%f (In radians)", Num1, Num2, ans);
                                                   ans = ans * 180/PI;
                                                   printf("\n%f (In degrees)", ans);
                                                   Sleep(250);
                                               }
                                               } else if (K == 2){
                                                   printf("Enter your value: ");
                                                   scanf("%f", &CheckF);
                                                   if (CheckF < -1 || CheckF > 1){
                                                       printf("Value cannot be higher than 1/lower than -1. Please recheck your input!\nERROR 1 Sintek\n\a");
                                                       system("pause");
                                                       system("cls");
                                                       } else {
                                                           ans = (acos ( CheckF ));
                                                           printf("Cosine inverse %f = \n%f (In radians)", CheckF, ans);
                                                           ans = ans * 180/PI;
                                                           printf("\n%f (In degrees)", ans);
                                                           Sleep(250);
                                                       }
                                                       } else if (K != 1 || K != 2) {
                                                           printf("Your input option is not found! Error 404\a\n");
                                                           system("pause");
                                                           system("cls");
                                                       }
                                             } else if (N == 9){
                                                 printf("[1] Entering opposite and adjacent of triangle\n[2] Entering the value directly\n\nYour option: ");
                                                 scanf("%d", &K);
                                                 if (K == 1){
                                                    printf("Enter opposite and adjacent sides of the triangle: ");
                                                    scanf("%f%f", &Num1, &Num2);
                                                    CheckF = Num1 / Num2;
                                                    ans    = (atan ( CheckF ));
                                                    printf("Tangent inverse %f/%f =\n%f (In radians)", Num1, Num2, ans);
                                                    ans = ans * 180/PI;
                                                    printf("\n%f (In degrees)", ans);
                                                    Sleep(250);
                                                                   } else if (K == 2){
                                                                       printf("Enter your value: ");
                                                                       scanf("%f", &CheckF);
                                                                       if (CheckF < -1 || CheckF > 1){
                                                                           printf("Value cannot be higher than 1/lower than -1. Please recheck your input!\nERROR 1 Sintek\n\a");
                                                                           system("pause");
                                                                           system("cls");
                                                                           } else {
                                                                               ans = (atan ( CheckF ));
                                                                               printf("Tangent inverse %f =\n%f (In radians)", CheckF, ans);
                                                                               ans *= 180/PI;
                                                                               printf("\n%f (In degrees)", ans);
                                                                               Sleep(250);
                                                                           }
                                                                           } else if (K != 1 || K != 2) {
                                                                               printf("Your input option is not found! ERROR 404\a\n");
                                                                               system("pause");
                                                                               system("cls");
                                                                           }
                                                                           } else {
                                                                               printf("Your input option is not found! ERROR 404\a\n");
                                                                               system("pause");
                                                                               system("cls");
                                                                           }
                                                                       }
            if (Opt == '3'){
            printf("Welcome to Fibonacci Series Mode.\n\nPress any key to stop while printing the numbers, to pause.\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(75);
                    printf("[2] 1\n");
                    Sleep(1075);
                }
                if (N == 3){
                    printf("[1] 0\n");
                    Sleep(75);
                    printf("[2] 1\n");
                    Sleep(75);
                    printf("[3] 1\n");
                    Sleep(1075);
                }
                if (N > 3){
                    printf("[1] 0\n");
                    Sleep(75);
                    printf("[2] 1\n");
                    Sleep(75);
                }
                while ( N > 3 && F <= N ){
                    xy = x + y;
                    printf("[%.0d] %.5g\n", Numbering, xy);
                    Sleep(75);
                    x = y;
                    y = xy;
                    F++;
                    Numbering++;
                    while (kbhit()){
                        printf("\n\n[+] Interrupted\n\nE to exit\nC to continue printing\n\nOption: ");
                        scanf(" %c", &H);
                        if (H == 'E'){
                            printf("Exiting in 3 seconds. Goodbye!");
                            Sleep(3000);
                            exit(0);
                            } else if (H == 'C'){
                                continue;
                            }
                    }
                }
                Sleep(1000);
                }
            
            
        if (Opt == '4'){
            srand(time(NULL));
            Choice = rand()%3;
            if (Choice == 0){
                a = rand()%5001;
                b = rand()%5001;
                c = a + b;
                printf("What is %d + %d?\nYour answer: ", a, b);
                scanf("%d", &d);
                if (d == c){
                    printf("Yes. You are right; Congratulations\n\n");
                    system("pause");
                    } else {
                        printf("No. The correct answer is %.0d. Need to practice more!\n\n", c);
                        system("pause");
                        system("cls");
                    }
                }
            if (Choice == 1){
                a = rand()%5001;
                b = rand()%5001;
                c = a - b;
                printf("What is %d - %d?\nYour answer: ", a, b);
                scanf("%d", &d);
                if (d == c){
                    printf("Yes. You are right; Congratulations\n\n");
                    system("pause");
                    } else {
                        printf("No. The correct answer is %.0d. Need to practice more!\n\n", c);
                        system("pause");
                        system("cls");
                    }
                }
            if (Choice == 2){
                a = rand()%20;
                b = rand()%20;
                c = a * b;
                printf("What is %d times %d?\nYour answer: ", a, b);
                scanf("%d", &d);
                if (d == c){
                    printf("Yes. You are right; Congratulations\n\n");
                    system("pause");
                    } else {
                        printf("No. The correct answer is %.0d. Need to practice more!\n\n", c);
                        system("pause");
                        system("cls");
                    }
                }
            }

        if (Opt == '5'){
                printf("Thank you for using my calculator. Hope to see you again!!");
                Sleep(1250);
                system("cls");
                exit(0);
            }
            
            if (Opt < '1' || Opt > '5'){
            printf("Your option is not found! ERROR 404");
            printf("\a\n");
            system("pause");
            system("cls");
                }
        }
}
\$\endgroup\$
  • 6
    \$\begingroup\$ Welcome to Code Review. I hope you'll get some nice reviews, as there are certainly some Pythonic elements to your code that have more elegant C counterparts. I took the liberty to change your description to highlight that your calculator already works fine, I hope you're pleased with the new description. If not, feel free to edit your post :) \$\endgroup\$ – Zeta Jun 29 at 13:25
18
\$\begingroup\$

Welcome to Code Review, we wish you the best.

General Observations
Congratulations on getting this to work, one function that is 356 lines long and almost 17K of text is a bit large and very hard to code and debug.

Complexity
The function main() is too complex and should be broken into functions. A general rule of thumb in all programming is that a function should only be one edit screen in size, because it is to hard to keep track of everything that is going on. Typical software / program design involves breaking the problem up into smaller and smaller pieces to make it easier to understand. An example would be having different functions handle the each of the modes listed in the menu, one for Special Functions, one for Fibonacci Series one for Normal maths operations and one for Random Mathematical Question. Each of these top level functions can call sub functions.

There are a few software principles involved here:

DRY programming, AKA Don't Repeat Yourself
In a large function such as main there will be code that repeats itself in different places, rather than repeating the code put that code into a function and call that function is necessary, this allows the code to be written and debugged once and that speeds up development and maintenance.

Single Responsibility Principle
There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

The KISS principal which is Keep it Simple
This is fairly self explanatory, make the code as simple as possible within small blocks.

The overall design principal is Top Down Design or Stepwise Refinement, this is applied generally to procedural languages, Top Down and Bottom Up design can be used in Object Oriented Design.

An example of one function could be print_menu_return_option().

char print_menu_return_option()
{
    char Opt;                          /*   Declaring the type variables     */
    int input_check = 0;

    while (input_check == 0)
    {
        printf("\n\nWhich mode do you want to use?\n[1] Normal maths operations\n[2] Special Functions\n[3] Fibonacci Series\n[4] Random Mathematical Question\n[5] Exit\n\nYour input: ");
        input_check = scanf(" %c", &Opt);
    }

    return Opt;
}

In the above code you should use the return value of scanf() to check for errors in user input.

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Given that the user is a beginner, I'd add some remarks about switch/case for the menu and submenus. Other than that full ACK on the review. \$\endgroup\$ – Zeta Jun 29 at 16:49
  • 1
    \$\begingroup\$ I did think about it, but I'd prefer to do it if the user posts a follow up which I strongly suggest. \$\endgroup\$ – pacmaninbw Jun 29 at 16:52
  • \$\begingroup\$ The while loop in print_menu_return_option() is interesting. Under what input or conditions would scanf(" %c", &Opt); ever return 0 to cause another iteration? I'd expect only 1 or EOF. Perhaps this is only template code for further building? \$\endgroup\$ – chux - Reinstate Monica Jun 30 at 21:31
5
\$\begingroup\$

Many reviews, so some additional ideas.

Programmable precision

Rather than hard-code the 5, use a flexible variable. Perhaps code later may want to allow the user to adjust the width.

// printf("Here is your answer:\nLg(%f) = %.5f (to 5 decimal places)\n\n", Num1, ans);
int prec = 5;
printf("Here is your answer:\nLg(%.*f) = %.*f (to %d decimal places)\n\n", 
    prec, Num1, prec, ans, prec);

Also consider "*.g" instead, more informative with small and large values.

float v. double

Little reason to use float, especially since code calls so many double functions. In C, double is the default floating-point type. Save float for code needing restrictions in space or tight performance.

Defensive coding

if (Num1 < 0){ is a good idea, yet deserves to be before ans = log(Num1);

Consider a test before divide:

if (Num2 == 0.0) Handle_div_by_zero();
else ans = (Num1 / Num2);

Machine pi

Although OP did well with at least 17 digits for pi, more digits do not hurt. Recommend for such popular constants, use twice the expected digit need or the common define when available. When FLT_VAL_METHOD == 2 (double math using long double), the greater precision is then employed.

#ifdef M_PI
#define PI M_PI
#else
#define PI ‭3.1415926535897932384626433832795‬
#endif

Trig functions and degrees

Trig functions, especially with large degree angles, benefit with range reduction in degrees first.
Why this sin cos look up table inaccurate when radian is large?

// ans = (cos ( Num1 * PI/180 ));
ans = (cos ( fmod(Num1,360) * PI/180 ));
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Why prec instead of precision? It took me a while to figure out what prec stood for. (There's definitely personal preference here though; I'd generally advocate against using abbreviations in the first place, except really common and trivial ones.) \$\endgroup\$ – tjalling Jul 1 at 9:27
  • \$\begingroup\$ @tjalling I tend to use a level of verbosity like the OP's with a leaning toward my preference. OP used a mixture with i, x, xy, y, Opt, Numbering, Oof, Check, etc. As prec is used in printf() as a precision (a C library term), I did not see a strong need to dominate the arguments with a lengthy precision, Num1, precision, ans, precision which hides a bit the key players of Num1, ans. As with such style issues, I recommend to follow your groups' style guide. \$\endgroup\$ – chux - Reinstate Monica Jul 1 at 21:01
  • \$\begingroup\$ Agreed on following the group/team practices. I mentioned it because variable naming can also be considered part of code review. But I should probably have made my own answer if I wanted to address that. \$\endgroup\$ – tjalling Jul 2 at 8:03
4
\$\begingroup\$

Whereas @pacmaninbw offers some excellent general strategies, let's look at some specific syntax.

Pi

This is a point of some contention, but - where a library defines M_PI, and most do - I tend to use it. Since you're including windows.h it's likely that you're using MSVC. In that case, it gives you math constants if you configure the compiler with _USE_MATH_DEFINES.

My opinion is that this is a precompiler directive, and the precompiler is build-configurable, so configure it in the build. If you need to port this to Linux, the precompiler configuration necessary to support M_PI would change, but you can cross that bridge when you get there.

Prototype

load();                    //Function Prototype

It is? For what function? I don't see this defined anywhere.

Variable declaration

Since C99, predeclaring all variables at the top of the function is both unnecessary and, I find, unsightly. You can declare and initialize these where they are used. Also, your naming (Oof?) needs a little love.

Implicit string concatenation

Split this:

    printf("\n\nWhich mode do you want to use?\n[1] Normal maths operations\n[2] Special Functions\n[3] Fibonacci Series\n[4] Random Mathematical Question\n[5] Exit\n\nYour input: ");

into

printf(
    "\n\n"
    "Which mode do you want to use?\n"
    "[1] Normal maths operations\n"
    "[2] Special Functions\n"
    "[3] Fibonacci Series\n"
    "[4] Random Mathematical Question\n"
    "[5] Exit\n\n"
    "Your input: "
);

Use a switch

This, and its related comparisons:

if (Opt == '1'){

should use a switch.

Order of operations

                                ans = ( log(Num2) / log(Num1));

does not need outer parens.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Code structure

Treat main.c as boss who calls other people up to do their job. In this case, functions where one function does one thing. main is doing everything here.

else{
 printf("No. The correct answer is %.0d. Need to practice more!\n\n", c);
 system("pause");
 system("cls");
}

This error message can be one function which receives one input c.

The whole block of if (choice == a_number) can go to one function with a better description. It makes maintaining the code much more easier. Debugging, extending functionality and having someone else read/ review your code becomes easy too.

Variables.

They can certainly have better names than alphabets. It reduces chances of unintentional editing. Describe what a variable stores: an incoming argument ? a return value ? indices ? a doesn't tell anything about what it stores. So whenever I encounter it, I'll have to check all the places where a was modified to see what it is doing.

clang-format

Please use clang-format to improve readability and consistent coding-style. Also it saves the manual effort of remembering to add indent or move braces etc.

| improve this answer | |
\$\endgroup\$

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.