trjhtr

I was asked to create an Advanced calculator in C, I call it advanced because it actually handles arithmetic rules.
It was mentioned that I don't need to handle code exceptions and that I should assume that the code has legal input.

It works as follows:

1. Enter a number and press enter


2. Enter an operator and press enter To


3. To finish user input you press X and then enter.

Again I mention this, I assume correct input was put in the program.

calc.h

#pragma once

/*Sort the equation, deal with multipliers/divisors then sub and add, load it all in one array then recalculate */

/* Includes ******************************************************************/
#include <stdio.h>
#include <Windows.h>
/* Macros ********************************************************************/

/* Types *********************************************************************/

/* Global Variables *********************************************************/

/* Function Declarations *****************************************************/

/*Accept the equation as input and sort*/
void sort(IN int *numbers, IN char *operators);

/*Return result based on operator*/
int getOp(IN int a, IN int b, IN char op);

/*Create equation*/
void createEqu(IN int *numbers, IN char *operators);

/*Prints the equation*/
void print(IN int *numbers, IN char *operators);

/*Calculate array sum*/
void calSum(IN int *sorted, IN int length);

calc.c

/* Includes ******************************************************************/
#include "calc.h"

/* Macros ********************************************************************/

/* Types *********************************************************************/

/* Public Function Definitions ***********************************************/

/* Global Variables **********************************************************/

/*Create equation*/
void createEqu(IN int *numbers, IN char *operators)

 /*Intialize equation creater, 0 get number, 1 get operator*/
 int currentOperation = -1;

 /*Iterate through the arrays*/
 int n = 0;
 int o = 0;

 /*While equation input was not ended by user*/
 while (currentOperation != 2)
 
 /*Number or operator*/
 if (currentOperation == -1)
 
 printf("Enter numbern");
 scanf(" %d", &numbers[n]);

 /*If operator is negative, turn number into negative*/
 if (o != 0 && operators[o - 1] == '-')
 
 numbers[n] = getOp(numbers[n], NULL, operators[o - 1]);
 
 n++;
 
 else
 
 printf("Enter operatorn");
 scanf(" %c", &operators[o]);
 o++;
 

 currentOperation = currentOperation*-1;
 /*check if the last operator was X to terminate equation input*/
 if (operators[o - 1] == 'X')
 
 currentOperation = 2;
 
 

 /*Each array is terminated with NULL so it would be easy to read through them*/

 operators[o-1] = NULL;
 numbers[n] = NULL;


/*Prints the equation*/
void print(IN int *numbers, IN char *operators)

 int i = 0;
 for (;operators[i] != NULL; i++)
 printf("%c", operators[i]);


/*Return result based on operator*/
int getOp(IN int a, IN int b, IN char op)

 if (op == '*')
 return a * b;

 if (op == '/')
 return a / b;

 if (op == '+')
 return a;

 if (op == '-')
 return -a;


/*Calculate array sum*/
void calSum(IN int *sorted, IN int length)

 int i = 0;
 int finalRes = 0;

 for (; i <= length; i++)
 
 printf("%d ", sorted[i]);
 finalRes += sorted[i];
 

 printf("%d", finalRes);


/*Accept the equation as input and sort*/
void sort(IN int *numbers, IN char *operators)

 int sorted[256];

 /*Iterators of arrays*/
 int s = 0;
 int n = 0;
 int o = 0;

 /*While expression is not over*/
 while (operators[o] != NULL)
 
 /*If operation is + or - then store integers in sorted array for later calculation*/
 if (operators[o] == '+' 

 calSum(sorted, s);


/* Private Function Definitions **********************************************/

main.c

/* Includes ******************************************************************/
#include "calc.h"

/* Function Definitions ******************************************************/
INT wmain(IN SIZE_T nArgc, IN PCWSTR *ppcwszArgv)

 int numbers[256];
 char equ[256];

 createEqu(&numbers, &equ);
 sort(numbers, equ);
 getch();

 /* Succeeded */
 return 0;

1. 
   

   Poor use of NULL. In operators[o] != NULL, NULL is a null pointer constant best used with pointers. operators[o] is a char, not a pointer. Suggest below:

   
   
   

   // while (operators[o] != NULL)
   while (operators[o])
   // or 
   while (operators[o] != '')
   

   
   


2. 
   

   Similar problem with NULL below. Do not use NULL to indicate the null character.

   
   
   

   // getOp(numbers[n], NULL, operators[o - 1]);
   getOp(numbers[n], '', operators[o - 1]);
   
   // numbers[n] = NULL;
   numbers[n] = '';
   

   
   



3. void print(IN int *numbers, IN char *operators) is strange in that it never uses numbers. Certainly incorrect functionality.




4. Highly suspicious about the correctness of the value of s in calSum(sorted, s);. Such terse variable loses clarity as to it role.

Minor

10. " " in scanf(" %d", &numbers[n]); serve no purpose. Code can be simplified to scanf("%d", &numbers[n]); and retain the same functionality.




11. Code that does not exceed the presentation width is more clear and easier to re-view. (Code should not need horizontal scroll bars.)



12. 
    

    I'd expect const for pointer to data that is not modified by the code. const better conveys code's intent and allows for select optimizations.

    
    
    

     // void print(IN int *numbers, IN char *operators)
     void print(const int *numbers, const char *operators)
    

    
    



13. The role of IN is not defined and remains unclear.

Strange use of currentOperation in calc.c: it is a state variable. Its values should be states that should have been defined with #defines.

currentOperation = currentOperation*-1;

This changes the state from the initial state -1 to the "work-in-progress" state 1 and later from state 1 to state -1. Why not use assignment to make it clear? For example:

#define STATE_0 -1
#define STATE_1 1
#define STATE_END 2

getOp: that is a strange name "get operator/operation" where it calaculates an experssion. Shouldn't calc or so be a better name?

Also, this function sees + and - as unary and / and * as binary. Where/how are binary + and - handled?