4
\$\begingroup\$

In the last few day, I wanted to sharpen my C skills a bit, so I wrote a math expression interpreter that can handle + - * / ( ) and only integers.

I just want to know how good this code is, if anyone has any comments on any part of the code and if writing such code guarantees that I understand every aspect of the C language.

Please note that this is my first project in C and this code is based on some variations of the shunting yard algorithm.

aux.c

#include <stdio.h>
void banner()
{
    //non important
}
void credits()
{
   // also non important to be mentioned here
}
void help()
{
    puts("Allowed command");
    puts("\tbanner  : view the interpreter's banner");
    puts("\tclear   : clear the screan");
    puts("\tcredits : view credits ");
    puts("\thelp    : view this help");
    puts("\tq       : quit\n");
    puts("allowed operations");
    puts("\t+   : add or positive");
    puts("\t-   : subtract or negative");
    puts("\t*   : multiply");
    puts("\t/   : divide");
    puts("\t%   : mod");
    puts("\t^   : power");
    puts("\t( ) : priority brackets\n");
    puts("max input length is 100 character");
}

aux.h

void banner();
void credits();
void help();

handle.c

#include <stdio.h>
#include "stack.h"
#include "parse.h"
int isempty(char *s)
{
    if (s[0] == '\0')
    {
        puts("Error: Expected non-empty expression");
        return 1;
    }
    return 0;
}

int endofexpression( char *s, operand **num, operation **op, int *end )
{
    if(s[*end] == '\0')
    {
        while(!operationempty( *op ) )
        {
            int a = pop_num(num), b = pop_num(num);
            int y = operate( a, b , pop_operation(op) );
            push_num(y,num);
            (*end)++;
        }
        return 1;
    }
    return 0;
}

I wanted to separate the parsing step from the interpreting step but I found it will be complicated so I just did this with one function as it appears in handle.c.

handle.h

int isempty(char *s);
int endofexpression( char *s, operand **num, operation **op, int *end );

main.c

#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include "parse.h"
#include "sio.h"
#include "aux.h"
int main()
{
    banner();
    char *exp = (char*) malloc (sizeof(char) * MAX );

    //exp stands for expression

    while(1)
    {
        printf(">>");
        //getting input
        if(get_s( exp ) == 1 ) continue;

        //handling basic commands
        if(strcmp(exp, "q")==0) return 0;
        else if(strcmp(exp, "banner")==0) banner();
        else if(strcmp(exp,"help")==0) help();
        else if(strcmp(exp, "credits")==0) credits();
        else if(strcmp(exp,"clear")==0) system("clear");
        else
        {
            int sol;
            if(parse(exp, &sol ) ==0) printf( "%i\n" , sol );
        }
    }
}

Makefile

all: main.o aux.o handle.o parse.o sio.o stack.o
    cc aux.o handle.o main.o parse.o sio.o stack.o -o simcalc -lm
main: main.c
    cc -c main.c -o main.o
aux: aux.c
    cc -c aux.c -o aux.o
handle: handle.c
    cc -c handle.c -o handle.o
parse: parse.c
    cc -c parse.c -o parse.o
sio: sio.c
    cc -c sio.c -o sio.o
stack: stack.c
    cc -c stack.c -o stack.o
debug:
    cc aux.c main.c handle.c sio.c stack.c parse.c -o simcalc -lm -Wall -g -ggdb3
clean:
    rm -f main.o aux.o handle.o parse.o sio.o stack.o

parse.c

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <math.h>
#include "stack.h"
#include "parse.h"
#include "handle.h"
// this function give the periority to each group of operators
int priority(char op)
{
    switch (op)
    {
        case '+':
        case '-': return 0;
        case '*':
        case '/':
        case '%': return 1;
        case '^': return 2;
        default : return -1;
    }
}

//this function tests which of two fuctions have high priority
int tpriority(char op1 , char op2 )
{
    int p1 = priority(op1), p2 = priority(op2);
    if(p1 == -1 || p2 == -1) return -1;
    else if(p1>p2) return 1;
    else if(p2>p1) return 2;
    else return 0;
}
// this function converts string number of defined length
//into int intiger
int stoi(char *number, int len)
{
    int i;
    char temp[len];
    for(i=0;i<len;i++) temp[i] = number[i];
    return atoi(temp);
}

//this function takes 2 integers and operation and perform it
int operate (int y, int x, int z)
{
    switch (z)
    {
    case '+': return x+y;
    case '-': return x-y;
    case '*': return x*y;
    case '/': return x/y;
    case '%': return x%y;
    case '^': return pow(x,y);
    default : return 0;
    };
}

int parse( char *inst, int *sol )
{
    //define the 2 stacks one for operators and one for operand
    operation *op = NULL;
    operand *num = NULL;
    int len = strlen(inst), start = 0, end = 0;
    // test for empy expression, or expression ends with operator,
    // or begin with operator, this is only done once during parsing 
    if(priority(inst[len-1])!=-1)
    {
        printf("Error: Unexpected operator \'%c\'\n",inst[len-1]);
        mkoperandempty(&num);
        mkoperationemtpy(&op);
        return 1;
    }
    if(priority(inst[0])>0)
    {
       printf("Error: Unexpected operator \'%c\'\n",inst[0]);
        mkoperandempty(&num);
        mkoperationemtpy(&op);
        return 1;
        }
    if(isempty(inst)) return 1;
    while(end<=len)
    {
        //handles brackets
        if(inst[end] == ')')
        {
            puts("Error: Unbalanced or unexpected parenthesis or bracket");
            mkoperandempty(&num);
            mkoperationemtpy(&op);
            return 1;
        }
        if(inst[end] =='(')
        {
            start=end+1;
            int bracketcounter =1;
            while(bracketcounter != 0)
            {
                end++;
                if(inst[end] == '(') bracketcounter++;
                else if(inst[end] == ')' ) bracketcounter--;
                if(end ==len)
                {
                    puts("Error: Unbalanced or unexpected parenthesis or bracket");
                    mkoperandempty(&num);
                 mkoperationemtpy(&op);
                    return 1;
                }
            }   
            inst[end] ='\0';
            int temp;
            if(parse(&inst[start],&temp)!=0)
            {
                mkoperandempty(&num);
            mkoperationemtpy(&op);
                return 1 ;
            }
            push_num( temp ,&num);
            start=end;
            end++;
            continue;
        }
    //handles end of expression
    if( endofexpression( inst, &num, &op, &end ) ==1 ) break;
    //handle digits
    else if( isdigit(inst[end]) )
    {
        start =end;
        while( isdigit( inst[end] ) ) end++;
        push_num( stoi(&inst[start] , end-start) , &num );
    }
    /***********************************************************************/
    else
    {
        //check if the operator is invalid
        if( priority(inst[end]) == -1 )
        {
            printf("Error: Undefined function or variable \'%c\'\n",inst[end] );
            mkoperandempty(&num);
         mkoperationemtpy(&op);
            return 1;
        }
        // unary positive and plus overriding 
        if(inst[end] == '+' &&(end ==0 || priority(inst[end-1]) !=-1 ) )
        {
            end++;
            start =end;
            if( priority( inst[end] )!= -1 )
         {
            printf("Error: Unexpected operator \'%c\'\n",inst[end]);
            mkoperandempty(&num);
            mkoperationemtpy(&op);
            return 1;
         }
            while( isdigit( inst[end] ) ) end++;
            push_num( stoi(&inst[start] , end-start) , &num );
            continue;
        }
        // unary negative and minus overriding
        else if(inst[end] == '-' &&(end ==0 || priority(inst[end-1]) !=-1 ) )
        {
            end++;
            start = end;
            if( priority( inst[end] )!= -1 )
         {
            printf("Error: Unexpected operator \'%c\'\n",inst[end]);
            mkoperandempty(&num);
            mkoperationemtpy(&op);
            return 1;
         }
            while( isdigit( inst[end] ) ) end++;
            push_num( -1*stoi(&inst[start] , end-start) , &num );
            continue;
        }

        else if( operationempty( op ) ) push_operation ( inst[end] , &op );
        else
        { 
            if( priority( inst[end-1] )!= -1 )
            {
                printf("Error: Unexpected operator \'%c\'\n",inst[end]);
                mkoperandempty(&num);
             mkoperationemtpy(&op);
                return 1;
            }
            while(!operationempty( op ))
            {
                if( tpriority( op->operation, inst[end] )>1 ) break;
                int a = pop_num(&num), b = pop_num(&num);
                int y = operate( a, b , pop_operation(&op) );
                push_num(y,&num);           
            }
            push_operation(inst[end], &op);
        }
        end++;
    }
}
    *sol = pop_num(&num);
    return 0;
}

parse.h

int parse(char *inst, int *sol);
int operate (int y, int x, int z);

sio.h

#include <stdio.h>
#include <ctype.h>
#include "sio.h"

// get_s is a fuction that takes adress to char array and
// fill it character by character the maximum number of
// character can be determined by the macro MAX

int get_s(char *s)
{
        int i=0;
        do
    {
        while( ( s[i] = getchar() ) != '\n' ) // stop reading if new line is reached
            {
            if(isspace(s[i]) ) continue;
                    if(i == MAX)
                    {
                            puts("Expression too long\ntype \"help\" to get help");
                            neglect_input();
                            return 1;
                    }

                    i++;
        } 
            s[i] ='\0'; // terminate the string
    } while(s[0] =='\0');
        return 0;
}
//neglect_input is a fuction that reads input from
//user until '\n' and store it in no where.
void neglect_input()
{
    while( getchar() != '\n' );
}

sio.h

#define MAX 100
// get_s is a function that takes adress to char array and
// fill it character by character the maximum number of
// character can be determined by the macro MAX

int get_s(char *s);
void neglect_input();

stack.c

#include<stdlib.h>
#include "stack.h"

int push_num(int x,operand **num)
{
    operand *temp = (operand*) malloc(sizeof(operand));
    if(temp==NULL) return 1;
    temp->number = x;
    temp->pervious = *num;
    *num = temp;
    return 0;
}

int pop_num(operand **num)
{
    int temp_num = (*num)->number;
    operand *temp = *num;
    *num = (*num)->pervious;
    free(temp);
    return temp_num;
}

int operandempty(operand *num)
{
    if (num == NULL )
    return 1;
    else return 0;
}
void mkoperandempty(operand **num)
{
    while(!operandempty(*num))
    {
        pop_num(num);
    }
}
int push_operation(char c, operation **op)
{
    operation *temp = (operation*) malloc(sizeof(operation));
    if(temp == NULL) return 1;
    temp->operation = c;
    temp->pervious = *op;
    *op = temp;
    return 0;
}
int pop_operation(operation **op)
{
    operation *temp_op = *op;
    char temp = temp_op->operation;
    *op = (*op)->pervious;
    free(temp_op);
    return temp;
}
int operationempty(operation *op)
{
    if (op == NULL ) return 1;
        else return 0;
}
void mkoperationemtpy(operation **op)
{
    while(!operationempty(*op))
    {
        pop_operation(op);
    }
}

stack.h

//stack for storing numbers 
typedef struct operand
{
   int number;
   struct operand *pervious;
} operand;
//stack for storing single characters 
typedef struct operation
{
    char operation;
    struct operation *pervious; 
} operation;
//all push funtion return 1 if malloc failed
//and 0 if malloc succeeded
// all pop functions make no empty stack checks
//all isXempty return 1 if stack is empty and 
// 0 otherwise 
int push_num(int x,operand **num);
int pop_num(operand **num);
int operandempty(operand *num);
void mkoperandempty(operand **num);

int push_operation(char c, operation **op);
int pop_operation(operation **op);
int operationempty(operation *op);
void mkoperationemtpy(operation **op);
\$\endgroup\$
  • \$\begingroup\$ the Makefile fails to allow for when a header file gets modified, the one or more of the source files will need to be recompiled. \$\endgroup\$ – user3629249 Mar 18 '15 at 21:14
  • \$\begingroup\$ Please do not use tabs in source code. it messes up the formatting, making the code much harder to read (and different editors/programmers use different lengths for the tab setting.) best to replace all tabs with spaces before saving a file, posting code. \$\endgroup\$ – user3629249 Mar 18 '15 at 21:15
  • \$\begingroup\$ Perhaps they were left off the posted code, but the header files seem to be missing the 'include' guards #ifndef ... #define .... #endif \$\endgroup\$ – user3629249 Mar 18 '15 at 21:16
  • \$\begingroup\$ please be consistent about the format of words in the code. 'camel' casing the variables and function names, all caps for #define names, etc is expected and greatly helps the readability of the code. \$\endgroup\$ – user3629249 Mar 18 '15 at 21:20
  • \$\begingroup\$ in modern C, a struct should not be 'typedef'd. that just clutters the code and obsecures what is actually being performed. \$\endgroup\$ – user3629249 Mar 18 '15 at 21:21
3
\$\begingroup\$

A Makefile is intended to only compile those files in a project that need re-compiling (thereby minimizing the amount of time needed to rebuild a project.

One of the kinds of files that will drive a recompile operation is a change to a header file. Then all files that depend on that header file need to be recompiled.

Therefore, in the Makefile, each source file that uses #include for header files, needs to have each of those header files listed as a dependency for compiling that source file. Typically written like this:

file.o: file.c header1.h header2.h

The following is a typical Makefile. It is using 'sed' to develop the dependency files. However, gcc (and similar compilers) have parameters that can do the same thing.

Note the expectation, in this that each source file has a corresponding header file. This file is written for and may need minor tweaking for other OSs

This file is written in such a manner that will take minimal modification to be used on other projects.

SHELL = /bin/sh


BINDIR  :=  /home/user/bin


.PHONY: all
all : $(BINDIR)/$(name) 


#
# macro of all *.c files 
# (NOTE:
# (the following 'wildcard' will pick up ALL .c files
# (like FileHeader.c and FunctionHeader.c 
# (which should not be part of the build
# (so be sure no unwanted .c files in directory
# (or change the extension
#
SRC := $(wildcard *.c)
OBJ := $(SRC:.c=.o)
DEP := $(SRC:.c=.d)
INC := $(SRC:.c=.h)


MAKE    :=  /usr/bin/make

CC      :=  /usr/bin/gcc

CP      :=  cp

MV      := mv

LDFLAGS :=  -L/usr/local/lib

DEBUG   :=  -ggdb3

CCFLAGS :=  $(DEBUG) -Wall -W

# pre processor flags, not C++ flags
#CPPFLAGS += =MD


LIBS    :=   -lssl -ldl -lrt -lz -lc -lm



#
# link the .o files into the executable 
# using the linker flags
# -- explicit rule
#
$(name): $(OBJ) 
    #
    # ======= $(name) Link Start =========
    $(CC) $(LDFLAGS) -o $@ $(OBJ) $(LIBS)
    # ======= $(name) Link Done ==========
    #



# note:
# using MV rather than CP results in all executables being re-made everytime
$(BINDIR)/$(name): $(name)
    #
    # ======= $(name) Copy Start =========
    sudo $(CP) $(name) $(BINDIR)/.
    # ======= $(name) Copy Done ==========
    #



#
#create dependancy files -- inference rule
# list makefile.mak as dependancy so changing makfile forces rebuild
#
%.d: %.c 
    # 
    # ========= START $< TO $@ =========
    $(CC) -M $(CPPFLAGS) $< > $@.$$$$;                      \
    sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.$$$$ > $@;     \
    rm -f $@.$$$$
    # ========= END $< TO $@ =========



# 
# compile the .c file into .o files using the compiler flags
# -- inference rule
#
%.o: %.c %.d 
    # 
    # ========= START $< TO $@ =========
    $(CC) $(CCFLAGS) -c $< -o $@ -I. 
    # ========= END $< TO $@ =========
    # 



.PHONY: clean
clean: 
    # ========== CLEANING UP ==========
    rm -f *.o
    rm -f $(name).map
    rm -f $(name)
    rm -f *.d
    # ========== DONE ==========




# include the contents of all the .d files
# note: the .d files contain:
# <filename>.o:<filename>.c plus all the dependancies for that .c file 
# I.E. the #include'd header files
#
# wrap with ifneg... so will not rebuild *.d files when goal is 'clean'
#
ifneq "$(MAKECMDGOALS)" "clean"
-include $(DEP)
endif
\$\endgroup\$
2
\$\begingroup\$

A header file, normally, should not be included more than once in a single source file. Otherwise the compiler will complain about duplicate declarations, etc.

The way to add 'include guards' to a header file is:

first line of file contains:
#ifndef SOME_UNIQUE_NAME_H

where the some unique name is usually the same as the name of the header file

the second line of file contains:
#define SOME_UNIQUE_NAME_H

this definition is visible though the whole compile operation so the header lines betwee the #ifndef and a final #endif will only be included once, no matter how many times that file is reference in #include statements.

the last line of file contains:
#endif  // SOME_UNIQUE_NAME_H

where the trailing comment is for documentation purposes only

\$\endgroup\$
0
\$\begingroup\$

regarding typedef'ing struct definitions.

example from posted code:

//stack for storing numbers 
typedef struct operand
{
   int number;
   struct operand *pervious;
} operand;

This just obscures/clutters the code and the clutters compiler namespace

the preferred method of defining a struct is:

//stack for storing numbers 
struct operand
{
   int number;
   struct operand *pervious;
};

then when ever that struct needs to be referenced:

struct operand  operation;

or

x = malloc( sizeof struct operand );
\$\endgroup\$
  • 2
    \$\begingroup\$ Why did you post several separate answers when you could have said all of it in a single one? \$\endgroup\$ – glampert Mar 19 '15 at 1:58
0
\$\begingroup\$

'camel casing' means that the first letter of each word in the name is capitalized, except for the very first word. This makes the words much easier to read.

This word formatting is used for functions names and variable names.

Names created with #define are normally ALL CAPS and utilize underscores '_' between words for readability (see the SOME_UNIQUE_NAME_H in my prior answer for an example).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.