please find out the bug in this infix to post fix program

[manohar]

//Program to convert infix to postfix

#include<iostream.h>
#include<stdio.h>
#include<stdlib.h>
#include<conio.h>
#include<string.h>

class infix
{
char infix[20],stack[10],postfix[20];
public:
infix(); //To initialize private members
void accept(); //To accept input expression
int check(); //This is the function to push elements to stack and
postfix
int check1(char ); //To check whether operator or operand
int diff(int ,int ); //To check whether input expression is valid or
not
void push2stack(char ); //To push elements to stack
int precedence(char ); //Evaluating the precedence of operators
char poppreced(); //To pop elements from stack in case of precedence
void popfstack(char ); //To pop elements from stack
void push2post(char ); //To push the elements to postfix expression
void display(); //To display postfix expression
};

infix::infix()
{
memset(stack,0,sizeof(stack));
memset(postfix,0,sizeof(postfix));
stack[0]='#'; //setting first element of array stack as '#'
}

void infix::accept()
{
cout<<"\nEnter the expression: ";
gets(infix);
infix[strlen(infix)]='#'; //adding '#' at end of array infix
}
int infix::check()
{
char *ptr=infix;
int i=0,cnt=0,cnt1=0,j;

for(i=0;i<strlen(infix);i++)
{
j=check1(*ptr);

switch(j)
{
case 0:
{
cnt++;
//cout<<"\n1."<<*ptr;
push2stack(*ptr);
ptr++;
}
break;

case 1:
{
char *ptr1=&stack[strlen(stack)-1];
cnt++;
//cout<<"\n2."<<*ptr;
popfstack(*ptr1);
ptr++;
}
break;
case 2:
popfstack(*ptr);
break;
case 3:
{
cnt1++;
//cout<<"\n3."<<*ptr;
push2post(*ptr);
ptr++;
}
break;

default:
;
};

}

if(diff(cnt,cnt1)%2==0)
return 0;
else return 1;
}

int infix::check1(char ch)
{
if(ch=='(') return 0;
else if(ch=='+') return 0;
else if(ch=='-') return 0;
else if(ch=='^') return 0;
else if(ch=='*') return 0;
else if(ch=='/') return 0;
else if(ch==')') return 1;
else if(ch=='#') return 2;
else return 3;

}
int infix::diff(int cnt,int cnt1)
{
if(cnt>cnt1)
return (cnt-cnt1);
else if(cnt<cnt1)
return (cnt1-cnt);
else return 0;
}

void infix:ush2stack(char ch)
{
if(stack[0]!=NULL)
if(precedence(ch)<=precedence(stack[strlen(stack)-1]))
{
char ch1=poppreced();
push2post(ch1);
}
else
stack[strlen(stack)]=ch;
}

int infix:recedence(char ch)
{
if(ch=='+' || '-')
return 0;
else if(ch=='/' || '*')
return 1;
else if(ch=='^')
return 2;
else
return 3;
}

char infix:oppreced()
{
char ch=stack[strlen(stack)-1];
stack[strlen(stack)-1]=0;
return(ch);
}

void infix:opfstack(char ch)
{
char *ptr=&ch;
if(ch!='#')
{
while(*ptr=='(')
{
push2post(*ptr);
stack[strlen(stack)-1]=0;
ptr--;
}
stack[strlen(stack)-1]=0;
}
else
{
ptr=&stack[strlen(stack)-1];
while(*ptr!='#')
{
push2post(*ptr);
stack[strlen(stack)-1]=0;
ptr--;
}
stack[strlen(stack)-1]=0;
_exit(0);
}
}

void infix:ush2post(char ch)
{
postfix[strlen(postfix)]=ch;
cout<<endl;
for(int i=0;i<strlen(postfix);i++)
cout<<postfix;
}

void infix::display()
{
cout<<endl;
for(int i=0;i<strlen(postfix);i++)
cout<<postfix;
}

void main()
{
infix a1;
clrscr();
a1.accept();
if(!a1.check())
cout<<"\nInvalid expression";
else
a1.display();

}

 

[manohar]

yeah it is not showing any warning also, how main returns int here.

okay anyway, i have tried by giving many inputs but some where bug is
there i am unable to find it....


1.what the program does?
A.when we give input expression as infix it will convert that
expression in to postfix.

2.what you want it to do?
A.i want to do the same as above.

 

[Jerry Coffin]

(e-mail address removed)>, (e-mail address removed)
says...

First a warning: asking about "the bug" carries the utterly false
implication that the code contains only one bug.

#include<iostream.h>
#include<stdio.h>
#include<stdlib.h>
#include<conio.h>
#include<string.h>

<iostream.h> is a pre-standard header that should NOT be used in any
new development. Only what I'd estimate to be a fairly small amount
of code has an excuse to continue using it at all.

In C++, there's rarely a really good reason to use <stdio.h>

<conio.h> isn't a standard header at all.

<string.h> contains declarations of the C string-handling stuff. If
you're dealing with strings in C++, you're usually better off

class infix

'infix' strikes me as a rather poor name for a class. It does little

{
char infix[20],stack[10],postfix[20];

These undocumented magic numbers may or may not qualify as a bug
(they're probably not the bug you're chasing anyway), but they're
clearly not what anybody would call very good code either...

public:
infix(); //To initialize private members
void accept(); //To accept input expression
int check(); //This is the function to push elements to stack and
postfix
int check1(char ); //To check whether operator or operand
int diff(int ,int ); //To check whether input expression is valid or
not
void push2stack(char ); //To push elements to stack
int precedence(char ); //Evaluating the precedence of operators
char poppreced(); //To pop elements from stack in case of precedence
void popfstack(char ); //To pop elements from stack
void push2post(char ); //To push the elements to postfix expression
void display(); //To display postfix expression
};

Most of these should almost certainly be private. They also
illustrate the unsuitability of 'infix' as the name of the class. At
first glance, it appears to me that the name of the class could be
something like 'infix2postfix', with its only public members being
operator() and possibly a ctor.

infix::infix()
{
memset(stack,0,sizeof(stack));
memset(postfix,0,sizeof(postfix));
stack[0]='#'; //setting first element of array stack as '#'

We can see what you assigned -- a meaningful comment would tell us
_why_ you did that...

gets(infix);

Whether it's the one you're looking for, this really IS a bug. I'm
not quite sure I'd go quite so far as to say you should _never_ use
gets, but you certainly shouldn't use it here.

infix[strlen(infix)]='#'; //adding '#' at end of array infix

This could well be the source of the bug you're looking for -- this
overwrites the string terminator.

int infix::check()
{
char *ptr=infix;
int i=0,cnt=0,cnt1=0,j;

for(i=0;i<strlen(infix);i++)

This is likely to unduly damage efficiency -- it re-measures the
length of the string for each iteration of the loop, turning an O(N)
loop into an O(N*N) one instead.

{
j=check1(*ptr);

switch(j)
{
case 0:

This completely obscures what you're doing. The name 'check1' tells
us essentially nothing about what the function does. The return
values (some unidentified integers) give us no clue either.

int infix::check1(char ch)
{
if(ch=='(') return 0;
else if(ch=='+') return 0;
else if(ch=='-') return 0;
else if(ch=='^') return 0;
else if(ch=='*') return 0;
else if(ch=='/') return 0;
else if(ch==')') return 1;
else if(ch=='#') return 2;
else return 3;

}

Even after looking at this, I can't suggest a good name for it,
because it's not at all clear what it's supposed to accomplish.

int infix::diff(int cnt,int cnt1)
{
if(cnt>cnt1)
return (cnt-cnt1);
else if(cnt<cnt1)
return (cnt1-cnt);
else return 0;
}

Is there some specific reason to avoid something like:
return abs(cnt1-cnt);
?

void infix:ush2stack(char ch)
{
if(stack[0]!=NULL)

While this works, it's really a poor idea -- 'NULL' is supposed to be
a null _pointer_, and shouldn't really be used for other purposes,
such as a NUL character.

[ ... ]

void infix:opfstack(char ch)
{
char *ptr=&ch;
if(ch!='#')
{
while(*ptr=='(')
{
push2post(*ptr);
stack[strlen(stack)-1]=0;
ptr--;
}
stack[strlen(stack)-1]=0;
}

Okay, you've got a really _serious_ bug here. ch is a _single_
character that has been copied to the stack. Using ptr as a pointer
to that character is a poor idea, but in itself allowable. Attempting
to modify, and then dereference that pointer, however, is a _major_
problem. When you called this function, you only copied ONE character
to the stack, but now you're trying to use multiple characters that
just aren't there.

Based on how you're using it, you should define popfstack to receive
a pointer to char, and when you call it, you'll pass the address of
(or in) the stack, so what it receives gives it access to the whole
stack instead of a single character.

else
{
ptr=&stack[strlen(stack)-1];
while(*ptr!='#')
{
push2post(*ptr);
stack[strlen(stack)-1]=0;
ptr--;
}
stack[strlen(stack)-1]=0;
_exit(0);

This is rather a problem as well -- when you're writing C++, you
should pretty much forget that ever even heard of the 'exit' or
anything very similar. While their use can be justified, it's only
rarely. Generally speaking, you should plan on returning from main --
usually via normal flow of control, otherwise via an exception.

}
}

void infix:ush2post(char ch)
{
postfix[strlen(postfix)]=ch;
cout<<endl;
for(int i=0;i<strlen(postfix);i++)
cout<<postfix;

It would also help if you indented this reasonably.

}

void infix::display()
{
cout<<endl;
for(int i=0;i<strlen(postfix);i++)
cout<<postfix;

This appears to be essentially the same as:
cout << "\n" << postfix;

}

void main()
{
infix a1;
clrscr();

clrscr() is non-standard. You're not getting much in exchange for
giving up portability.

 

[Alf P. Steinbach]

* manohar:

//Program to convert infix to postfix

Hi.

Jerry Coffin gave you a thorough analysis else-thread, showing that the code you
presented is a mess, with not just one bug.

The question is, how could you end up with such a mess?

And one common reason is that you've been trying to implement too much at a time.

Start with something *much* simpler.

Don't add more than 4 lines of code before compiling and re-testing.

For example, since even your 'main' routine is incorrect, start with trying to
write a valid C++ program that does *nothing*.

Only when this compiles cleanly (including with Comeau Online, check), add
output of "Hello, world!". This since even your basic output was incorrect. Make
sure that your program compiles fine both with your own compiler and with Comeau
Online, and that it does produce the expected text when run.

And so on.

One little step at a time -- one *very* tiny little step at a time.


Cheers & hth.,

- Alf

 

[manohar]

thanks to both of you jerry coffin and Alf P. Steinbach.......

you spent your valuable time on this and guided me in a better
way........