A queue based calculator for basic math operations

Currently, I am interested in programming. I have programmed a calculator in C. Everything works, but I do not feel completely confident in my solution.

Are there any logic or efficiency issues with my code?

Supported operations: \$+\$, \$-\$, \$*\$, and \$/\$.

Algorithm: Flowchart

main.c

#include "practice.h"

int main() {
    char arr[100];
    char c;
    int length = 0;
    int result;

    while((c=(getchar())) != '\n')
        arr[length++] = c;

    result = EquationToQueue(arr,length);
    printf("%d", result);
}

practice.c

```
#include "practice.h"

queue* make_queue(size_t capacity, enum data_type type) {
queue* new_queue;
new_queue = (queue*)malloc(sizeof(queue));
new_queue->top = 0;
new_queue->bottom = 0;
new_queue->capacity = capacity;
new_queue->type = type;
new_queue->element = (queue_element)malloc(sizeof(queue_element) capacity);

return new_queue;
}
void push_queue(queue* object_queue, enum data_type type, ...) {
va_list ap;
va_start(ap,type);

if(object_queue->top >= object_queue->capacity) {
printf("Over %d is impossible!", object_queue->capacity);
exit(0);
}

switch(type) {
case queue_INT :
object_queue->element[object_queue->top++].data.val_i = va_arg(ap,int);
break;
case queue_CHAR :
object_queue->element[object_queue->top++].data.val_c = (char)va_arg(ap,int);
break;
default :
printf("Unknown type in push_queue()\n");
exit(0);
}
va_end(ap);
}
void pop_queue(queue object_queue, void return_val) {
if (object_queue->top == object_queue->bottom) {
printf("queue is empty!\n");
exit(0);
}

switch(object_queue->type) {
case queue_INT :
(int)return_val = object_queue->element[object_queue->bottom++].data.val_i;
break;
case queue_CHAR :
(char)return_val = object_queue->element[obj

I see a number of things that may help you improve your code.

Add error checking

Data input usually requires a lot of careful consideration for most software. Checking for malformed input and handling it gracefully often takes a considerable amount of thought, time and code. In this code we have a 100 character input buffer, but nothing prevents a user from inputting more that that. The result is a classic stack buffer overflow vulnerability. The while loop input should check for (and prevent!) a buffer overflow.

Perform input sanitation

Closely related to error checking, but slightly different, is to sanitize user input. For example, if the user enters the string "3 + 2 5" (with spaces), the rather surprising result is 247394 on my machine, rather than the expected answer of 13 which is returned when the same equation is entered as "3+25" (without spaces). It's usually better to adjust user input for such things, or at least throw an error, but to simply give a wrong result is usually unacceptable.

Isolate the internal representation from the interface

The code uses a variadic function push_queue and requires ` to implement it, but I would note that the interface (that is, the portion in the "practice.h" file) only requires (for the definition of size_t) and doesn't require either or listed there. Instead, those two should be moved to the practice.c file. The implementation details should be isolated to the extent possible. This will make it easier to modify the code without needing to change calling code and provides for a clear and minimal interface specification.

Fix the bug

If the user enters "12/2/3" the expected result is 2 but in fact what happens is that the program evaluates 2/3 first (as integers) and gets a result of 0. It then tries to evaluate 12/0 which results, on my machine, in a floating point exception error (divide by zero) and a crash. Evaluating this expression from left to right (and also checking for and handling divide by zero) would be a better way to handle this.

Check the return value of malloc before dereferencing

The primary way that the operating system uses to tell your program that it's running out of memory is to return NULL when the program requests a memory allocation. For that reason, you must check the return value before dereferencing it to make sure it's not NULL.

Use include guards

There should be an include guard in the .h file. That is, start the file with:

#ifndef PRACTICE_H
#define PRACTICE_H
// file contents go here
#endif // PRACTICE_H

Be careful with assumptions about size_t

The current code includes capacity which is defined as a size_t type, which is a good choice for that variable. However the code also includes this line:

printf("Over %d is impossible!", object_queue->capacity);

The problem with this, is that the %d specifier expect an int and size_t may or may not be an int. In fact, on my 64-bit Linux machines, it's a long int and so the compiler complains. The code can be written to accommodate either by using the z prefix for the argument; that is, make it %zd or %zu to have the compiler automatically use an appropriate type for size_t.

Separate input, output and calculation

To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of your program (which does not depend on the underlying OS). For example, instead of having various error messages and calls to exit() scattered within functions such as push_queue and pop_queue, it might be better to have those functions return an error code that the caller can then decide how to handle. For example, if you ever wanted to make this into a GUI program without console I/O, then the current code would not work as well as if the code were divided as I suggest.

Eliminate "magic numbers"

In a number of cases, the code uses "magic numbers" such as 10 and 50 that have no obvious meaning. These would be better as named constants. In some cases, the limit seems quite arbitrary, and might reasonably need to be altered by a user of the code.

Don't leak memory

This code has a delete_queue function, but it's never called and the result is therefore a memory leak. That should be fixed (by deleting the two allocated queues in EquationToQueue) so that the code can be reliably reused.

Consider better naming

Although it uses queues internally, the EquationToQueue function is misnamed. What it really does is to create a single integer evaluation of the passed numeric equation. The fact that it uses queues or any other particular data structure is, and probably should be, irrelevant to the user. For that reason, I'd suggest renaming it to something like EvaluateEq

StackExchange Code Review Q#139619, answer score: 11