Organize your data
I see one struct
, the rest of your data is stored in several arrays, and pointers to pointers are passed around to access them. Try to organize your data even more. For example, information about the tokens, like name, precedence and association, could be grouped in a struct
:
typedef struct {
const char* name;
int precedence;
bool right_assoc;
} TokenInfo;
static const TokenInfo token_infos[] = {
[PLUS] = {"PLUS", 0, false},
[MINUS] = {"MINUS", 0, false},
…
};
I would also create a struct TokenStack
and a struct Line
, so we can pass the stack and line via a single pointer. These struct
s could not only hold the actual data, but also members that indicate where the top of the stack and the current position in the line is.
Avoid using overly generic names
Avoid using names that are very generic, like get()
and next()
. In a larger program, this increase the chance of name conflicts.
Make more use of the standard library
There is a lot of functionality in the standard library that you can make use of, avoiding reinventing the wheel. For example, instead of is_blank()
, use isspace()
. Instead of treating a string buffer as a stream, consider passing a FILE*
to eval()
and have it use getch()
, ungetc()
and so on. This can also be used on string buffers by the way, by using the POSIX fmemopen()
function, although that is not standard C.
Consider implementing a Pratt parser
Your parser works, but adding more operations will increase the complexity. You also have to manually manage a stack. Consider implementing a Pratt parser instead; it will result in cleaner and more easily extensible code. In particular, it handles the difference between prefix and infix operators very well, which brings me to the following:
Negation should be done with -
It's very annoying to have to use _
as opposed to -
to negate a number. No mainstream programming language needs that. You should be able to modify your code to handle that case, by examining the top of the stack when handling MINUS
: if it's empty or the top is not an INT
, treat it as a NEG
instead.
Avoid hardcoded buffer sizes
Hardcoding the line length to 256 seems fine for the simple example expressions you are testing, but if this is going to be used in production, there will come a time when someone uses it for an even larger expression. Your current code might in the best case crash, in the worst case it will continue running but will have unexpected results because it is reading and writing out of bounds.
Either avoid hardcoding buffer sizes, and make them resize when necessary, or at the very least check whether you are about to go past the end of a buffer, in which case you should handle this situation gracefully; for example by printing an error message to stderr
and exiting with EXIT_FAILURE
.
Note that this is one of the drawbacks of C. If you like the language in general but would like to avoid most of the memory issues, consider using C++ in combination with the standard containers.