Advanced C calculator

Question

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 number\n");
 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 operator\n");
 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] == '+' || operators[o] == '-')
 {
 /*Save both original numbers */
 sorted[s] = numbers[n];
 sorted[s + 1] = numbers[n + 1];
 s++;
 n++;
 o++;
 }
 else
 {
 /*calculate mandatory expression result and store in next cell value*/
 numbers[n + 1] = getOp(numbers[n], numbers[n + 1], operators[o]);
 n++;
 o++;
 /*If last operation was mandatory by arithmetic rules (div or mul), and its the last operation store result in sorted array*/
 if (operators[o] == NULL)
 {
 sorted[s] = numbers[n];
 }
 }
 }
 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;
}

• chux: Clearly state and add the review goals.
• chux: Where is IN defined?

Answer 1

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] != '0円')
   

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], '0円', operators[o - 1]);
   // numbers[n] = NULL;
   numbers[n] = '0円';
   

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.

• Potato: Thank you very much for your feedback! IN indicates INPUT while OUT indicates OUTPUT, these are the conventions i had to use.
• Potato: the space in the scanf invokes it to ignore the previous '\n'
• chux: @Potato The space if not needed to ignore the previous '\n'. scanf("%d", &numbers[n]); will ignore leading white-space, even without a space in the format. Hence, " " in scanf(" %d", &numbers[n]); serves no purpose.

Answer 2

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?