Conversion from postfix to infix in C

Just over half way through in K&R, came across an exercise to convert from postfix ("reverse-Polish") notation to infix (standard) notation.

Basically I'm posting here to see if there is anything excessive in my code, or any way to reduce the amount of storage it takes up. It runs extremely fast and doesn't take up much space, but if space can be reduced I believe it always should be.

Also, I have tried to maintain the clearest readability and consistent style throughout my code, but I am new to C, so if there are certain style conventions I am breaking, let me know as well.

```
#include
#include
#include

#define MAXLEN 1000 // Maximum string length

int isOperator(int c);
void addPrecedence(char * infixExpr);
void pushExpression(char * infixExpr);
char * popExpression(void);

int main(int argc, char * argv[])
{
// Only one argument is allowed
if (argc != 2) {
printf("Usage: evaluate [expression] from postfix to infix\n");
return 1;
}

int i;
char * expression = argv[1];
char left[MAXLEN], right[MAXLEN], expr[MAXLEN];

for (i = 0; expression[i] != '\0'; i++) {
if (isalpha(expression[i]) || isdigit(expression[i])) {
// Copy character into a string so it can be pushed onto the stack
expr[0] = expression[i], expr[1] = '\0';
pushExpression(expr);

} else if (isOperator(expression[i])) {
strcpy(right, popExpression());
strcpy(left, popExpression());

expr[0] = expression[i], expr[1] = '\0';
strcat(left, expr);
strcat(left, right);
addPrecedence(left);
pushExpression(left);

} expr[0] = '\0'; // Reset expr character array
}
printf("%s\n", popExpression());
return 0;
}

// Check if the character is a valid operator
int isOperator(int c)
{
int i;
char legalOps[] = {'+', '-', '*', '/'};

for (i = 0; i = 0; i--)
infixExpr[i+1] = infixExpr[i];

Since you are using end-of-line commands, I will assume that you are using at least C99 in this review (it may be a compiler extension to C89 though).

main

There are too many things in your main function. It shouldn't handle the expression parsing, but delegate this task to another function. Also, if you are using C99, you don't need to return 0; at the end of main: the compiler will have your program automagically return 0 when it reaches the end of the function.

isOperator

Whenever it makes sense, try to use boolean values instead of integers. The header ` defines bool, true and false that you can use to make your code clearer. Using booleans instead of integers can help to clarify your intent. For example, here is your function isOperator with booleans instead of integers

bool isOperator(int c)
{
    int i;
    char legalOps[] = {'+', '-', '*', '/'};

    for (i = 0; i < 4; i++)
        if (c == legalOps[i])
            return true;
    return false;
}

By the way, there are many other way to implement this function. Your current function uses a loop and therefore performs in \$ O(n) \$ where n is the number of operators while you could rewrite it to perform in \$ O(1) \$, with a lookup table for example. That said, it shouldn't be a problem since you only have 4 operators.

Variable declarations

You seem to declare your variables at the beginning of blocks. It is not needed anymore with C99, you can declare your variable when you need it. You can also declare a variable in a for` loop and therefore write things like this:

for (int i = 0; infixExpr[i] != '\0'; i++) {
    // ...
}

StackExchange Code Review Q#51483, answer score: 4