Please help me in self documenting the program below.
All you have to do is, take a quick glance at the following code to see if you can understand what each part is doing. If you have any difficulty in understanding(at a quick glance that is!), please tell me, so that I can add a comment.
Also tell me if I need to change the order in which I defined the functions and classes to enhance readability!
I've already posted this question once at
http://www.daniweb.com/forums/thread102405.html
I was asked to make a lot of changes so I closed the thread, made the changes and is reposting the question.
/* Expression Evaluation Notebook
Program to read text from keyboard and write it into a file, after
evaluating all simple mathematical expressions in the text.* /
#include<fstream.h>
#include<conio.h>
#include<stdio.h>
#include<process.h>
#include<ctype.h>
/**********************Definitions of Data Structures Used*********************/
union element //used to store parsed elements
{ char sym; //for symbol
int ival; //for integer
float fval; //for float
};
struct info //information part of stack/queue
{ element el; //element
char flag; //flag to show type of element
};
class Queue
{ struct node
{ info inf;
node *next;
}*front,*rear;
public:
Queue()
{front=rear=NULL;}
void push(info n);
info pop();
info peek()
{return front->inf;}
};
class Stack
{ struct node
{ info inf;
node *next;
}*top;
public:
Stack()
{top=NULL;}
void push(info);
info pop();
info peek()
{return top->inf;}
};
/******************************End of definitions******************************/
//Defining Global objects and variables
fstream temp;
Queue expr;
int exp_flag;
/***************************Function declarations******************************/
char symb( char);
int insert();
void post_convert();
int evaluate();
int expression();
/****************************End of declarations******************************/
/*******************************Main Function**********************************/
void main()
{ clrscr();
temp.open("temp.mth",ios::out|ios::in);
char ch;
cin.get(ch);
while(ch!=7) //Read till ^G is encountered
{ exp_flag=0;
if(ch=='=')
{ cin.get(ch);
if(ch=='=') //If '==' is encountered,
exp_flag=expression(); //evaluate expression
else //if its a single '=', put into file
temp.put('=');
if(exp_flag) //if expression evaluation fails
cout<<"Error : Expression syntax error!!";
}
else //if no '=' is encountered, just write to file
temp.put(ch);
cin.get(ch);
}
getch();
}
/********************************End of Main***********************************/
/************************Other Function definitions****************************/
int expression() //converts infix expression to postfix and evaluates
{ exp_flag=0;
int fail=insert(); //to read expression, parse it and insert to
//queue
if(!fail)
{ post_convert(); //convert infix to postfix
fail=evaluate();
}
if(fail)
{ char in[81];
if(exp_flag==1) //Remove input expression if
while(temp.get()!=';'); //evaluation failed
return -1;
}
return 0;
}
int insert() //parse expression and insert into queue
{ int ctr=0,sign=1,MinusFlag=1; //flags to handle parantheses
//and unary minus
float factor;
char in[81];
info inf={ 0,0}; //element is by default integer
for(int i=0;i<81&&in[i-1]!=';';i++)
{ cin.get(in[i]); //read the expression without
if(isspace(in[i]))i--; //white spaces
}
for(i=0;in[i]!=';';i++) //parsing
{
if(symb(in[i]))
{
if(in[i]=='-'&&MinusFlag)
//If operator is unary
{ sign*=-1; //minus, change sign of
inf.flag=0; //'sign', set flag as
continue; //integer, and continue
}
else sign=1; //if symbol is not unary minus,
//reset 'sign' to 1
//check if operators and operands
//are entered alternately
if(symb(in[i])&&(symb(in[i+1])||in[i+1]==';'))
if(in[i+1]!='('&&in[i+1]!='-'&&in[i]!=')')
{ cout<<"Error!! too many operators!!!";
return -1;
}
if(in[i]=='(')
{ ctr++;
if(inf.flag!=2&&i!=0) //If there is no symbol
{ inf.el.sym='*'; //before '(', then
inf.flag=2; //multiply by default
expr.push(inf);
}
}
else if(in[i]==')')
ctr--;
if(ctr<0) //check if parantheses are balanced
{ cout<<"Error!!! More right paranthesis!!!";
return -1;
}
if(in[i+1]=='-') MinusFlag=1;
inf.flag=2; //element is a symbol(operator)
inf.el.sym=in[i];
expr.push(inf);
}
else if(isdigit(in[i]))
{ MinusFlag=0;
if(symb(in[i-1])||i==0) //if first digit of a number,
{ inf.flag=0; //reset flag
inf.el.fval=0.0; //reset el
inf.el.ival=(int)(in[i]-'0')*sign;
}
else if(inf.flag) //if float
{ factor/=10; //decrement the place value
//of last digit
inf.el.fval+=(in[i]-'0')*factor*sign;
}
else //if integer
inf.el.ival=10*inf.el.ival+(int)(in[i]-'0')*sign;
if(symb(in[i+1])||in[i+1]==';')
expr.push(inf);
}
else if(in[i]=='.') //if '.' is encountered, set flag to float
{ inf.el.fval=(float)inf.el.ival;
inf.flag=1; //INPUT is float type
factor=1; //Reset the place value
//of last digit added
if(symb(in[i+1])||in[i+1]==';')
expr.push(inf);
}
}
if(ctr>0) //if more left parantheses, balance with
while(ctr) //right parantheses in the end of expression
{ info inf={')',2};
expr.push(inf);
ctr--;
}
temp.write(in,i); //write the expression into file
exp_flag=1;
return 0;
}
char symb(char c) //function to set the precedence order
{ switch(c)
{ case '^':return 9;
case '/':return 8;
case '*':return 8;
case '+':return 7;
case '-':return 7;
case '(':return -1;
case ')':return 1;
default :return 0;
}
}
void post_convert()
{ Queue temp;
Stack symbols;
info psym,pexp={'(',2};
symbols.push(pexp); //push ')' into symbols stack
while(pexp.flag!=-5) //Till the end of expr queue
{ pexp=expr.pop();
if(pexp.flag==2) //if element is symbol,
{ psym=symbols.peek();
while((symb(psym.el.sym)>=symb(pexp.el.sym))
&&pexp.el.sym!='(')
{ psym=symbols.pop(); //push all operators of
temp.push(psym); //greater precedence
psym=symbols.peek(); //into expr
}
if(pexp.el.sym==')') //If the symbol from expr is ')',
symbols.pop(); //then remove '(' from the
//symbols stack
else //Else push the symbol into
symbols.push(pexp); //the symbols stack
}
else if(pexp.flag==-5) //when elements in queue ends
{ psym=symbols.pop(); //pop all elements from symbols
while(psym.el.sym!='(') //stack and push into temp
{ temp.push(psym);
psym=symbols.pop();
}
}
else //else pexp is operand and hence
temp.push(pexp); //push into temp
}
expr=temp; //now expr holds the postfix expression
}
int evaluate() //function to evaluate the postfix expression
{ Stack eval;
info pexp,op1,op2,res;
pexp=expr.pop();
do
{ if(pexp.flag==1||pexp.flag==0) //if element is a number
eval.push(pexp); //push into stack
else if(pexp.flag==2) //if element is operator,pop
{ op2=eval.pop(); //last 2 elements from stack
op1=eval.pop();
if(op1.flag||op2.flag) //if op1 or op2 is float type
res.flag=1; //the result is float
else if(op1.flag==-5) //else if there is no element
cout<<"Too less operands"; //in stack, display error
else //else
res.flag=0; //the result is int type
switch(pexp.el.sym) //do the operation
{ case '+' :if(res.flag)
res.el.fval=
(op1.flag?op1.el.fval:op1.el.ival)+
(op2.flag?op2.el.fval:op2.el.ival);
else
res.el.ival=
(op1.flag?op1.el.fval:op1.el.ival)+
(op2.flag?op2.el.fval:op2.el.ival);
break;
case '-' :if(res.flag)
res.el.fval=
(op1.flag?op1.el.fval:op1.el.ival)-
(op2.flag?op2.el.fval:op2.el.ival);
else
res.el.ival=
(op1.flag?op1.el.fval:op1.el.ival)-
(op2.flag?op2.el.fval:op2.el.ival);
break;
case '*' :if(res.flag)
res.el.fval=
(op1.flag?op1.el.fval:op1.el.ival)*
(op2.flag?op2.el.fval:op2.el.ival);
else
res.el.ival=
(op1.flag?op1.el.fval:op1.el.ival)*
(op2.flag?op2.el.fval:op2.el.ival);
break;
case '/' :if(res.flag)
res.el.fval=
(op1.flag?op1.el.fval:op1.el.ival)/
(op2.flag?op2.el.fval:op2.el.ival);
else
res.el.ival=
(op1.flag?op1.el.fval:op1.el.ival)/
(op2.flag?op2.el.fval:op2.el.ival);
break;
default :cout<<"\nError!!in eval";return-1;
}
eval.push(res); //after operation, push the result back
//into stack
}
pexp=expr.pop(); //pop next element from queue
}while(pexp.flag!=-5); //until the end of queue
res=eval.pop(); //pop the last element in the eval stack
op1=eval.pop(); //check if it is indeed the last element
if(op1.flag!=-5)
return -1;
//display the result
cout<<"OUTPUT of expressions in the last line : "
<<(res.flag?res.el.fval:res.el.ival)<<'\n';
//write into file
temp<<'$'<<'#'; //flag to signal beg of expression result
temp.write((char*) &res,sizeof res);
temp<<'$'<<'#'; //flag to signal end of expression result
return 0;
}
/*******************************End of Definitions****************************/
/***********************Definition of Queue class functions*******************/
void Queue::push(info n)
{ if(front==NULL) //If queue has no elements
front=rear=new node; //start queue
else //If it has elements
{ rear->next=new node; //add node to the end
rear=rear->next;
}
if(rear==NULL) //if node cannot be created show error
{cout<<"Error cannot insert in Q";return;}
rear->inf=n; //add information in the node
}
info Queue::pop()
{ if(front==NULL) //if underflow, send info
{ info temp={0,-5}; //with flag set as -5
return temp;
}
//else return the element from the front and delete the node
info temp=front->inf;
node *temptr=front;
if(front==rear) front=rear=NULL;
else
front=front->next;
delete temptr;
return temp;
}
/*******************************End of Definitions****************************/
/*********************Function Definitions of Stack Class**********************/
void Stack::push(info n)
{ node* temp=new node; //create node
if(temp==NULL) //if node creation failed show error
{ cout<<"Cannot insert into stack!!!";
return;
}
temp->inf=n; //add information into node
temp->next=top;
top=temp; //update top
}
info Stack::pop()
{ if(top==NULL) //if underflow, return info with flag
{ info temp={0,-5}; //set as -5
return temp;
}
info n=top->inf;
node* temp=top;
top=top->next; //update top
delete temp;
return n; //return the info on top element
}
/********************************End of Definitions***************************/>If you have any difficulty in understanding(at a quick glance that is!)
Most of your comments are pointless. At a glance I can tell what the code is doing; any reasonably experienced programmer can do this easily. What I really want to know is why you're doing it. I can't extract your thought process from reading the code, and that's where comments really shine.
See this
http://www.daniweb.com/code/snippet491.html
I wrote it long back.. There are many typos but you can see how do I document header of a function or a file.
However, this time your documentation was better than last one that you posted.
@Narue
could you please illustrate the difference between the "why" comment and the "what" comment!
@ dubeyprateek
could you please explain what is to be written in "parameters" in your comment on functions.
Should I comment using the format
Method Name::
Parameters::
Return::
Purpose::
all the functions in my program.
also please tell me which of the comments I should remove - atleast some of them, if there are many(so that I may know what sort of comments is to be retained and what is to be removed!).
Self-document code is not only comments what the code does but also the concept of the code. To give you an idea of how to comment, I write a small example of code with comments. (I'm not a professional at commenting and I'm also not a good-english writer and speaker, forgive me if there are any mistake)
// The isprime() returns true if the passing
// number is prime number, and returns false if
// the number is not a prime. This function
// does not work with the negative number.
bool isprime(unsigned int num)
{
// 0 is definity not a prime number
if (num == 0) return false;
// The first three numbers(1, 2, 3) are the
// prime number, so no need to check.
if (num < 3) return true;
// Even numbers conver 50% of all the number,
// and 20% of the odd number are divisible by
// number 3. By checking whether the number
// is divisable by 2 and 3 would increase the
// performance speed by 60%
if (num % 2 == 0 || num % 3 == 0) return false;
// To check whether one number is prime number
// or not, we need to run from 2 till itself
// and check if it divisable by any number;
// however, we can minimize it by simply
// test it from only 2 till number/2 because:
// 2 * a < number (a MUST be smaller than
// number/2). Interestingly, every number can
// be form by 6k+1, 6k+2, 6k+3, 6k+4; 6k+5.
// However, 6k+2 and 6k+4 are divisible by 2
// and 6k+3 is divisible by 3 which leaves
// 6k+1 and 6k+5 which can be written as 6(k+1)-1.
// To gain the great advantage of this interest fact,
// we increase our increasment integer by 6
for (int i = 6; i < num/2; i+=6) {
// Whenever we increase our increasment number
// by 6, we check whether our number divisable by
// these number format, 6k+1 and 6k-1.
if (num%(i-1)==0 ||
num%(i+1)==0) return false;
}
// If there is no divisable number, then it is
// a prime number.
return true;
}>could you please illustrate the difference between
>the "why" comment and the "what" comment!
Gladly. This is a "what" comment:
/***********Main Function***********/
void main() All it does is waste space because even the most basic reader will know that this is the main function. You don't need to tell me that, but you do need to tell me why you chose to use void main instead of int main. This is a "why" comment (edited slightly from your code):
// if more left parantheses, balance with
// right parantheses in the end of expression
if(ctr>0)
while(ctr) You're not telling me what's going on (pushing closing parens until the counter reaches 0), you're telling me why you're doing it (balancing opening parens with closing parens).
>To give you an idea of how to comment, I write a small example of code with comments.
That's overkill, of course. If you find yourself commenting one line of code with more than one line of commenting, it's probably too much. Exceptionally obtuse code that requires lots of comments probably needs to be rewritten. You don't want to hide the code in reams of comments because that's just as bad for readability as having no comments. Here's something better:
// param "num": The number we want to check
// returns: true if the number is prime, false if not
bool isprime(unsigned int num)
{
// Handle the simple cases
if (num == 0) return false;
if (num < 3) return true;
if (num % 2 == 0 || num % 3 == 0) return false;
// Filter out numbers not of the form 6k+1 or 6k-1.
// Reference: http://primes.utm.edu/notes/faq/six.html
for (int i = 6; i < num/2; i+=6) {
if (num%(i-1)==0 || num%(i+1)==0)
return false;
}
// num wasn't filtered out, so it's prime
return true;
} Notice the clever use of external references for more detailed information. You don't need to write a technical paper to describe your algorithm, especially with such a well understood fact as this.
>however it is very self-documented.
I can't disagree with that. I can only assume that a wall of green text is "very" self-documented. It encourages me to read the code about as much as zero formatting, but certainly very self-documented. :icon_rolleyes:
>however it is very self-documented. I can't disagree with that. I can only assume that a wall of green text is "very" self-documented. It encourages me to read the code about as much as zero formatting, but certainly very self-documented. :icon_rolleyes:
Don't worry, I would never comment that much in my project, and the comments look nicer in gray colour than in green colour.
>and the comments look nicer in gray colour than in green colour.
I use purple for my comments. On a black background it's easier to focus on them if I need to but ignore them for the most part.
> also please tell me which of the comments I should remove
All of them?
Example:
else if(in[i]=='.') //if '.' is encountered
How is that comment adding anything to the understanding? Comments which essentially repeat the syntax of the language don't add anything. Anyone reading the code could write those comments, but it wouldn't help understand what was going on, or why.
If you're implementing a published algorithm, then I'd expect to see references.
Things like "how" it does it, "why" it's needed are also useful things to know.
Narue's post #9 is pretty much what I'd go for, though I'd personally make the function header comments Doxygen compatible.
The /********** comments for breaking up the sections of the module are good, but just too long for this forum.
I've made the changes you people asked me to make ( I hope!:) ). If its not enough please tell.
I have also made some changes to the program.
In addition to commenting please tell me if there is something that i can improve upon in the program, without greatly changing the structure of the program.
Please find attached the program "file1.cpp".
Salem's avatar told you all about it.
#include<conio.h>
#include<stdio.h> You are using C header files rather than C++ header files.
i'm using an old version of C++.
its just like C. Only difference - you can have classes in this.
And about the non-standart stuff .
i didn't find anything to that effect in Salem's post.(if you are reffering to post no. 14)
oh! sorry!
i didn't see your last post!
So you mean to say,
int main()
is the standard code???