Calculator in C with Stack and Linked List

Question

im trying to write a simple postfix calculator that reads the line and performs the calculation using a Stack. I was wondering if somebody coudl check if is struct's and push function are correnctly implmented. I would by very greatful for any advice or clues what I need to take in consideration. Im a Student of Informatics and that one of my first programms that i'd love to finish before Christmas :)

Can somebody tell me how to pull out just two elements form the top of the stack and not to traverse though whole stack :/

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <malloc.h>
#include <string.h>
#include <math.h>   // Definnes special Float - NAN ("Not A Number")

struct StackElement{
    int data;
    struct element *next;
  }StackElement;


    /*
   Typ stack_top is pointer to the StackElement
*/
typedef struct StackElement* stack_top{
    StackElement* first;

    }stack_top;


/*
    starc
   Lay Element on Stack

   If *stacktop points to NULL, there is no element in the Stack 
*/
void stack_push(stack_top *stacktop, float value)
{
    if(stacktop->first == NULL){
      StackElement->data = value;
      StackElement->next= NULL;
      stack_top->first= *StackElement;
      }

    else{
        StackElement->next = stack_top-> first;
        stack_top->first= *StackElement;  // New first element
        StackElement->data= value;
        }





/*
   Take one element from the stack

   If Stack is empty than set *stacktop = NULL

   , give float NAN back
*/
float stack_pop(stack_top *stacktop)
{
    if(&stacktop==NULL){
       printf("Stack ist leer \n")

    }
    // Hier Code einfügen
    return NAN;
}

/*
   Identyfy Token. Difference few cases:
   - Token is a nummber : lay it on the Stack.
   - Token is an Operator (+, -, *):
       1. Take the two elements from the top of the Stack.
       2. Use the operator.
       3. Lay the result back on the top of the stack.

   Implementiere hier die Rechenoperationen (+, -, *) und lege das
   Ergebnis zurück auf den Stack. Beachte, dass du mit Floatingpointwerten
   arbeitest, z.B. auch negative Kommazahlen.
*/
void process(stack_top *stacktop, char* token)
{   int a, operand1, operand2;
    assert(token != NULL);
    StackElement* temp = malloc(sizeof(StackElement));
    if ( char &token == + ||char &token== - || char &token == * ) == 0 )
    {
        stack_push(stacktop, token)

    else{}
        int a= &token;
       switch(a)
        {

            case '+':
                    result = operand1 + operand2;
                    break;
            case '-':
                    result = operand1 - operand2;
                    break;
            case '*':
                    result = operand1 * operand2;
                    break;

        }
    return result;
  }
}


const int MAX_LINE_LENGTH=1024;
/*
 main() reads the Input line by line and divides it single separate Tokens (linked list).
 Diese Funktion muss nicht geändert werden.
*/
int main(int argc, char** args)
{
    stack_top stacktop = NULL;
    char line[MAX_LINE_LENGTH]; // Input line
    char* token;                // Pointer the current token;

    while ( fgets(line, MAX_LINE_LENGTH, stdin) ) {
        token = strtok(line, " ");

        while (token != NULL) {
            process(&stacktop, token);  // perform Stackoperationen 
            token = strtok(NULL, " ");  // Read new Token
        }

        float result = stack_pop(&stacktop);    // Take Last result from Stack .

        if (stacktop != NULL) { // Mehr Operanden als benötigt angegeben. Fehler.
            while (stacktop != NULL) {
                stack_pop(&stacktop);   //Clean Stack 
            }
            printf("Error\n");
        } else if (result != result) {  // result ist NAN: Berechnung fehlgeschlagen  (z.b. zu wenig Operanden)
            printf("Error\n");
        } else {
            printf("%4.3f\n", result); // Gib Resultat aus
        }
    }
}

Answer 1

The StackElement

First, I think you meant to begin with a typedef your definition of the StackElement structure like:

typedef struct StackElement {
    int data;
    struct StackElement *next;
} StackElement;

Then, the next pointer which is a member of the StackElement structure type points to a StackElement itself: The next node in this singly-linked list. By the way, this self-referencing declaration is exactly why you cannot omit the structure's tag name and simplifying like this:

typedef struct { /* Unnamed structure here, no "StackElement" this time */
    int data;
    struct StackElement *next; /* Error here, StackElement tag name was never defined */
} StackElement;

By the way, we can see int data; chosen here but a float value assigned to it later.

The stack_top pointer

Next, something doesn't seem right with the following:

    /*
   Typ stack_top is pointer to the StackElement
*/
typedef struct StackElement* stack_top{
    StackElement* first;

    }stack_top;

I believe what you intended to do, also supported by your comment above the code, is declare a pointer to a StackElement, specifically one that will be used to always point to the top of the stack. In that case, since the StackElement type is already declared, you can simply do:

StackElement *stack_top; /* ... or StackElement *first;     */

If you really want to have another alias for the StackElement* type you can do: typedef StackElement* stackPtr; and then declare the actual variable thus:

stackPtr stack_top;

The process function

The process function has some obvious problems:

• It is a void function returning the value of an undeclared result variable. Considering specifying an int or float (whichever you really meant to use) as the return type of the function.

• In the condition part of the if statement, you need to surround the operator char values between single quotes like this: if (*token == '+'), etc.

• There is a problem with the curly braces in the if statement. On the true part, you don't really need to open a brace (which you forgot to close) since you have a single statement to be executed. Consider removing the { or closing it before the else part.

• You declare char &token a few times in the if condition part. I think you really just meant to use it like my example on the second bullet, above. Likewise, you declare int a twice, when you should only say a = *token; in the else section. By the way, if you really meant to declare a char reference (on the function parameter list), you should have probably tagged this question as C++. Then you would do a = token, respectively. EDIT: While I was writing this up a comment on your question appeared explaining more on the difference between &token and *token.

• In fact, having else {} the aforementioned statement and the switch statement that follows it are actually executed even if the token was not an operator.

• You are dynamically allocating memory for a StackElement but then never doing something with that pointer, temp, while it is in scope. After the function returns you have no way to refer to that memory again and it won't be freed until the program terminates.

This is but a part of the story...

These were some quick (or that was the idea for my first answer) observations on your code. I trust our community will be able to help even more and point out any other mistakes / improvements in your code (and probably mine, too). Good luck with your implementation of the calculator! :-)