# feedback for exercise 4-11 in K&R

I know that there is one more topic about the exercise 4-11, but the difference is that I solved this exercise and i just need some feedback on my solution. So, I'll explain how it works on an output like "3 4 +\n", for example. The static variable c is initialized with the value ' ' so the condition of the first if statement evaluates to true, the while loop within the if statement will run until c reaches the value '3'. Because it is a number, the function will return the NUMBER signal and the variable c will get the value ' ' - the value that breaks the loop within the 3rd if statement.

Now we are, again, on the first if statement and, again, c has the value ' '. The condition is evaluated to true and the loop within the if statement will run until c reaches the value '4'. Again, because '4' is a "number" the function will return the NUMBER signal and c will get the value '+' - the value that breaks the loop within the 3rd if statement.

This time c is equal to '+' and the condition of the first if statement is evaluated to false, so the value '+' will be returned. I saved the value of the variable c in a temporary variable tmp, and initialize tmp with the value of c before changing it to ' '.

Without this movement I'll get an infinite loop.

The program is made of 5 parts:

main.c

#include <stdio.h>
#include <stdlib.h>
#include "calc.h"
#define MAXOP 1001

int main() {
char entry;
char s[MAXOP];
double op2;

while((entry = getop(s)) != EOF) {
switch(entry) {
case NUMBER:
push(atof(s));
break;
case '+':
push(pop() +pop());
break;
case '*':
push(pop() * pop());
break;
case '-':
op2 = pop();
push(pop() - op2);
break;
case '/':
op2 = pop();
if(op2) {
push(pop() / op2);
}
else {
printf("can't divide by 0");
}
break;
case '\n':
printf("The value is: %f \n", pop());
break;
default:
printf("Unrecognized command %s\n", s);
break;
}
}

return 0;
}


getch.c

#include <stdio.h>
#define MAXBUF 100

static char buf[MAXBUF];
static int bufp = 0; /* next free position in buffer */

int getch(void) {
return (bufp > 0) ? buf[--bufp] : getchar();
}

void ungetch(int c) {
if(bufp < MAXBUF) {
printf("ungetch has been called\n");
buf[bufp++] = c;
}
else {
printf("the buffer is full\n");
}
}


stack.c

#include <stdio.h>
#define MAXSTACK 100

static double stack[MAXSTACK];
static int sp = 0; /* next free position in stack */

void push(double f) {
if(sp < MAXSTACK) {
printf("\t--> the value %f has been pushed\n", f);
stack[sp++] = f;
}
else {
printf("error: the stac is full\n");
}
}

double pop(void) {
if(sp > 0) {
return stack[--sp];
}
else {
printf("error: the stack is empty!\n");
return 0.0;
}
}


calc.h

#define NUMBER '0'
#define MAXLINE 1000

/*stack related functions */
void push(double f);
double pop(void);

/* output */
int getch(void);
void ungetch(int c);

/* filtration fuctions */
int getop(char s[]);


and getop.c

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

int getop(char s[]) {
int i, tmp;
static int c = ' ';

if((s[0] = c) == ' ' || c == '\t') {
while((s[0] = c = getch()) == ' ' || c == '\t')
;
}

s[1] = '\0';
if(!isdigit(c) && c != '.') {
tmp = c;
c = ' ';
return tmp;
}

i = 0;
if(isdigit(c)) {
while(isdigit((s[++i] = c = getch())))
;
}
if(c == '.') {
while(isdigit((s[++i] = c = getch())))
;
}

s[i] = '\0';

return NUMBER;
}

• In case anyone else is wondering, getop is being used to collect the next character or numeric operand. Here's the exercise: Modify getop so that it doesn't need to use ungetch. Hint: use an internal static variable. Commented Jan 12, 2014 at 12:56
• I realized that I actually don't need the last if statement. Commented Jan 12, 2014 at 15:00
• The rest of code was added. Commented Jan 13, 2014 at 14:03
• Whenever you see something obscure like s[0] = c = getch()) == in code, you know that someone has been reading too much K&R. Never use assignment inside if statements. It is dangerous practice and also makes the code less readable. There does not exist a single case where you ever need to do this. Commented Jan 20, 2014 at 7:37

Comments on getop() only:

Your initial loop repeats the exit condition twice. It (and subsequent code) also contains double assignments, which are unnecessary and generally better avoided:

if((s[0] = c) == ' ' || c == '\t') {
while((s[0] = c = getch()) == ' ' || c == '\t')
;
}


Here is a simpler version:

while (c == ' ' || c == '\t') {
c = getch();
}
s[0] = c;


Your loop to read a number is repeated and repetition is normally undesirable. The loop could be extracted into a function and called twice:

static int get_number(char *s)
{
int c = getch();
for (; isdigit(c); c = getch()) {
*s++ = c;
}
*s = '\0';
return c;
}


On end of file, this returns EOF.

Note that you should probably handle invalid input such as "1.2.3"

In main

char entry;
...
while((entry = getop(s)) != EOF) {


entry should really be an int