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***************************/

Recommended Answers

All 30 Replies

>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.

I know that my code is over-commented; however it is very self-documented.

>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.

commented: Purple, eww that's so girly. -2

> 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".

void main()
{	intro();
	menu();
}

Non-standard code.

sorry!
i didnot get you

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???

Of course. If I were you, I would seperate the code into 3 files: stack.h, queue.h, and main.cpp

oh! i too found the file too big to handle.
but we are not supposed to split the program code.
you see, its not in our syllabus!

He was referring to Salem's avatar which states that void main'ers are doomed. void main() , main should always return an int ie., int main()

its a joke right?

its a joke right?

Nope, people that use void main are doomed to be forsaken by the appropriate deity, if the case arises that they do not believe in one then they will be forced to watch nothing but reruns of Who's the boss for the rest of eternity.

its a joke right?

No it isn't.
http://c-faq.com/ansi/voidmain.html

If you ever get to the point of writing useful programs, don't screw up by writing void main and returning garbage to your users. I've been on the receiving end of such sloppy coding, and it's distinctly not funny.

That Programming tips were quiet useful!
But i couldnot understand certain terms in it.
Could someone explain to me what it means?
1. "telephone test"
2. off-by-one error

>1. "telephone test"
Read your code to someone over the phone. If they don't understand, you need to rewrite it.

>2. off-by-one error
Also known as a fencepost error, based on the simple logic question "If you build a fence 100 feet long with posts 10 feet apart, how many posts do you need?".

ok!
now what about the comments?
is it enough?should i add more? delete some? modify some?

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.