HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Primitive console string calculator in C

Submitted by: @import:stackexchange-codereview··
0
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: 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

-
#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.