patterncMinor
Primitive console string calculator in C
Viewed 0 times
consolecalculatorstringprimitive
Problem
Recenlty I have finished this primitive console string calculator in C. I need your balanced criticism, I'd like to know what I have to correct, remake and learn.
Permitted operations:
Sample input string:
main.c
stack.h
```
#define ELEMENTARY_SIZE_STACK 5 // initial the length of stack
#define RESIZE_ADDING 5 /* how much will be add when the stack size
will be equal to ELEMENTARY_SIZE */
#include
#define ValType double
#define ValType_IOSPECIF "%lf"
typed
Permitted operations:
+ - * / ( ) ^Sample input string:
11 * -10 + 15 - 35 / (35 - 11) + 2^2 =main.c
#include "stack.h"
#include
#include "calculating.h"
#include
#define EXPR_LENGTH 1000
main()
{
while (1)
{
Stack* nums = createEmptyStack();
ValType lastval, prevval, result;
char* temp = NULL, temp_char;
char num_buf[NUM_LENGTH], expr_buf[EXPR_LENGTH];
CalcErrors err_flag = NO_ERR;
system("cls"); // clear the screen
printf("Enter the expression: ");
/* Transform the infix notation to the reverse Polish notation and processing the error if necessary */
if (transformToRPN(expr_buf) != NO_ERR) {
err_flag = WRONGSYM_ERR;
goto break_loop;
}
temp = expr_buf; // to don't change the pointer expr_buf
/* Parsing the operands from expression in the reverse Polish notation */
while (temp_char = getOperand(num_buf, temp, &temp))
{
switch (temp_char) {
/* Processing the operators */
case SUB: case ADD: case MUL: case POW: case DIV:
if (nums->size size == 1 ? printf("= %f\n", pop(nums)) : printf(MSG_LACKOP); break;
case LACK_OPERAND: printf(MSG_LACKOP); break; // FIX THE TEXT
}
printf("\nDo you want to calculate something else? y/n > ");
getchar(); // skip the '\n' symbol
temp_char = getchar();
if (temp_char != 'y' && temp_char != 'Y')
break;
}
}stack.h
```
#define ELEMENTARY_SIZE_STACK 5 // initial the length of stack
#define RESIZE_ADDING 5 /* how much will be add when the stack size
will be equal to ELEMENTARY_SIZE */
#include
#define ValType double
#define ValType_IOSPECIF "%lf"
typed
Solution
Design
-
-
I would expect code guards in the .h files
-
Rather than allocating per the size of the type, allocate based on the size of the variable. Less error prone and easier to maintain.
-
Unclear why code is avoiding
-
Minor Stuff
-
Suggest
-
The test for a
-
Missing check for unary
-
Ensure code passes a value in the range of
-
As the return
-
For debugging, using
Quite a bit more exist to review, but GTG.
-
#define VAL 1 #define SUB '-' #define ADD '+' etc. are too generically named. Suggest a naming convention that does not so readily conflict with other code. Same for function names push(), resize(), pop(), peek(). In general, the .h files add name all over the name-space. Suggest a corner of it instead.-
I would expect code guards in the .h files
// or some uppercase variant
#ifndef stack_h
#define stack_h
....body of .h file
#endif-
Rather than allocating per the size of the type, allocate based on the size of the variable. Less error prone and easier to maintain.
//Stack* st = malloc(sizeof(Stack));
//st->data = malloc(sizeof(ValType)*ELEMENTARY_SIZE_STACK);
Stack* st = malloc(sizeof *st);
st->data = malloc(sizeof *(st->data) * ELEMENTARY_SIZE_STACK);-
Unclear why code is avoiding
-0. No need for that.if (temp_ch != '0') // to don't allow the '-0'-
getOperand() does not seem to allow .123 as input. I would expect this to pass.Minor Stuff
-
Suggest
bool return type for isOperator().-
The test for a
, to change it into . for atof() is interesting, yet a problem. atof() accepts a , or . depending on locale. I would not folding these two characters together without checking locale and then calling a locale sensitive function.-
Missing check for unary
+. Code check for unary -. Complete code would check for both.-
Ensure code passes a value in the range of
unsigned char to isspace(). Aside from EOF, passing negative value to isspace() is UB.// while (isspace(*ptr))
while (isspace((unsigned char) *ptr))-
As the return
ValType pop() may be different from double, ensure a matching type to %f.// printf("= %f\n", pop(nums))
printf("= %f\n", (double) pop(nums))-
For debugging, using
"%e" is more illuminating.// printf("= %f\n", (double) pop(nums))
printf("= %e\n", (double) pop(nums))Quite a bit more exist to review, but GTG.
Code Snippets
// or some uppercase variant
#ifndef stack_h
#define stack_h
....body of .h file
#endif//Stack* st = malloc(sizeof(Stack));
//st->data = malloc(sizeof(ValType)*ELEMENTARY_SIZE_STACK);
Stack* st = malloc(sizeof *st);
st->data = malloc(sizeof *(st->data) * ELEMENTARY_SIZE_STACK);if (temp_ch != '0') // to don't allow the '-0'// while (isspace(*ptr))
while (isspace((unsigned char) *ptr))// printf("= %f\n", pop(nums))
printf("= %f\n", (double) pop(nums))Context
StackExchange Code Review Q#124751, answer score: 9
Revisions (0)
No revisions yet.