Problems with Round Robin Algorithm

Question

1.31/5 (4 votes)

See more:

After,My previous post on trying to come up with a code for Round robin algorithm in c++ which can be found here
[^]

I thought of taking Andy's suggestion and come up with a new way of writing the algorithm. Thanks to Andy and the people of code project for their help.

So, here is my new code and i have added comments almost every where. I am stuck at some part of my program . Please can anyone review my code and correct my mistake. The areas where i am stuck, i shall mention below. Here is my code--

C++

#include <iostream>
#include <stdio.h>

static int n;
int i,j,total_time = 0;
float avgtat=0, avgwt = 0;
int queue[];
int time = 0, front = 0,rear = 0,q_count = 0; //q_counnt means the number of elements in the que
				
using namespace std;


struct data //structure contains all the details of the processes
{
	int at,st,ct,tat,wt; //at = arrival time, st= scheduled time, ct = completion time,
	                     //tat= turn around time, wt = waiting time
	char pname[20];
	float ntat;


}temp;

void push(int q) //push function basically pushes a current process in the que
	{
		queue[rear++] = q;
	}

	int pop() //to pop out an element from the top of the que
	{
		int x;
		x = queue[front++];
		return x;
	}

	void check(struct data a[]) //this is to check which processes have arrived
	{
		while(a[j].at<time&&j>		{
			q_count++; //number of process in the que is incremented by 1 
			push(j++); //pushing the element in the que and incrementing the j value
		}
	}



	void finding(struct data a[])
	{
		int temp_st[n] ,flag =0, count = 0, p_process; //temp_st is the temporary service time, flag is used to determine if a process is under
		                                             //execution ,p_process is the name of the current process
		j = 0;  //j is the global variable
		int tq;
		cout<<"Enter time quantum"<<endl;
		cin>>tq;
		for(i=0;i<n;i++)>
		{
			temp_st[i] = a[i].st; //service time of the processes is stored in the temporary service time
		}

		time = a[0].at; //this is to set the starting time as the arrival time of a[0] which is my first process
		q_count = 1; //number of elements in the que is 1
		push(j++); //pushing the first process in the que
		while(time<=total_time)  //time is less than the total time
		{
			if(flag==1 ||q_count!=0) //variable flag denotes that a process is selected and q_count != 0 is to denoted that atleast one process has arrived
			{
				if(flag==0 &&count==0) //condition to select the process
				{
					p_process = pop(); //to pop a process from the que
					count =0; //count is set to 0
					flag=1; //flag is set to 1 to identify that a process is under execution
				}

				temp_st[p_process]--; //we are decrementing the service time by 1
				
				if(temp_st[p_process]==0) //to check if the service time of the present process is equal to 0
				{
					time++; //time is incremented by 1
					count =0;
					a[p_process].ct = time; //this is to get the completion time of the present process
					flag = 0; //flag is set to 0
					q_count--; //the process is removed from the que 
					check(a); //check if any new process has arrived
					continue; //continue back to the top part of the loop
				}
				count++;

				if(count==tq) //if count is equal to time quantum
				{
					count = 0; //we make count set to zero again
					time++;  //time is incremented by 1
					check(a); //check if any process has arrived
					push(p_process); //push the current process back to the que
					flag =0; //flag is set to zero that is no process is under execution yet
				}

				else //if time count is not equal to time quantum
				{
					time++; //we increment time 
					check(a); //to check if any new process has arrived
				}
		}

	

	else  
	{
		time++;
		check(a);

	}

		}
	
	}


	void display(struct data a[])
	{  
		//Please help me here, i am confused what should i declare here
	}



int main()
{
	cout<<"Enter the number of processes"<<endl;
	cin>>n;

	struct data aa[n];


	//sorting
	for (i = 0;i<n;i++)>
	{
		cout<<"Enter the name of the processes"<<endl;
		cin>>aa[i].pname;
		cin>>aa[i].at>>aa[i].st;
	}

	for (i=0;i<n;i++)>
	{
		for(j=i+1; j<n;j++)>
		{
			if(aa[j].at<aa[i].at)>
			{
				temp = aa[i];
				aa[i] = aa[j]; 
				aa[j] = temp;
	            
			}
			else if(aa[i].at==aa[j].at) 
			{
				if(aa[j].st<aa[i].st)>
				{
					temp = aa[i];
					aa[i] = aa[j];
					aa[j] = temp;
				}
			}
		}

		//finding the total time

		total_time+=aa[0].at+aa[0].st;
		for(i = 0;i<n;i++)>
		{
         if(aa[i].at>total_time)
			 total_time += aa[i].st;
		}
		
		finding(aa);

	 return 0; 
	}

Here are the few problems i am having--

1. The part where i call the structure RR in my main
struct RR a[n];
//here it gives me error by saying that static n must have a constant value

2.Also in my void finding function where i declare an array called
int temp_st[n] to store the service time/burst time of each process. I get the same error which says "Static n must have a constant value"

3. Finally, i am new to data structures and i dont have much experience with it. So, i am confused what should i declare in my display function
void display(struct data a[]) { ------Please help me here -----}

Lastly please suggest if there is any more corrections which i should make. Thanks

Posted 27-Jul-14 1:22am

Surajit Das

Updated 27-Jul-14 6:34am

Paul Conrad

v3

Add a Solution

Comments

m.r.m.40 27-Jul-14 11:20am

Why didn't you wrap your code into the code blocks?
It really makes it easier to read the question.

Paul Conrad 27-Jul-14 12:38pm

As a side note, you can make your code more readable by clicking on where it says code above the editor box, and you can optionally pick C++ from the menu that appears. It helps making code readable.

Surajit Das 27-Jul-14 13:28pm

Thank you. I shall do that from next time.

Paul Conrad 28-Jul-14 1:40am

You're welcome! Makes it much easier to read!

nv3 27-Jul-14 15:25pm

This is basically C code, not C++. Nevertheless, I started to read your code and stumbled over the definition of "queue". You don't allocate space for it. So your program must crash anyway. Then, the while statement in the check function has a missing right parentheses. Obviously, you didn't even compile your code. And the wrong indentation makes even harder to read it.

If you want others to take some time and review your code, you should at least have the courtesy to cover the basics.

Surajit Das 27-Jul-14 17:12pm

With all due respect, everyone makes mistakes. If i wasted your precious time then i am sorry. You dont ever need to read my code again. And yes today i am a beginner in the field of programming. Give me few years and i wont need your help. Thank you

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Stefan_Lang · Accepted Answer · 2014-07-28T04:23:00

I went through the code and found the following issues:

1. Bad design: you're using lots of global variables needlessly. You shouldn't. While you are free to not use the object oriented paradigm, even procedural programming uses structs and function arguments to pass on variables and values instead of global variables. So why don't you use a struct that stores the current state of the processes and pass it to every function that manpulates it?

2. int queue[]; this array doesn't appear to be initialized anywhere. You can't access the elements of an array if you don't at first either specify it's dimensons, or later allocate it dynamically on the heap with sufficient size!

3. queue[rear++] = q;
The function push() does not check for the validity of the index rear. What is the maximum value? Of course you cannot access an element of the array queue past the last - but then you didn't allocate memory for the array to start with...

4. x = queue[front++];
Same issue with function pop()! You don't check for valid index values.

5. void check (struct data a[])
Missing size info: You're passing an array without information on the size of the array, and iterating over the elements without being sure they exist. In C, the usual way to be on the safe side is to pass the size of the array as a second argument. In C++ the better way is to use a standard container instead, e. g. std::vector

6. while(a[j].at<time&&j>
Apart from the fact this line is missing a closing parenthesis (probably just a mistype ) , is that condition really what you wanted? Especially the last bit? I honestly have no idea what you meant to accomplish here.

More importantly: what is the initial value of j? Why are you even using a global variable here? I can only repeat myself: do not use global variables! (unless you have a very good reason - and you don't one)

7. q_count++; //number of process in the que is incremented by 1
When was the last time you initialized q_count?

8. int temp_st[n]
This is not a valid definition of an array. The size of the array (n) must be constant and known at compile time. n is a variable, i. e. not constant, and therefore not known at compile time. The compiler cannot reserve memory for an array that it doesn't know the size of - even worse, it cannot possibly allocate memory for a size that may be different for consecutive calls to the function finding() !

9. for(i=0;i<n;i++)>
Looks like another typo at the end of the line.
Apart from that: here's yet another loop/function that assumes it knows the size of the array passed to the function. The only way to ensure this works is to keep track of every location in the code that calls this function! This may not be an issue for a small program like yours, but it is very, very bad once you start writing larger programs! Better not get used to that style f programming!

10. while(time<=total_time) //time is less than the total time
Ugh. Did I mention global variables and why you shouldn't use them? Who intialized total_time, and when? Is it initialized at all? Please don't do this - if you're reading unfamiliar code, the last thing you want is see a line like this and then have to check the entire code for occurences of a global variables and verify that it has been initialized. Or not!

11. count =0; //count is set to 0
This is just a pet peeve of mine: your effort to write comments is appreciated. However, this comment offers absolutely no information that the statement doesn't obviously provide already! if there isn't anything non-obvious you can add to clarify the need or purpose of a statement, just don't add a comment!

There are plenty more of such comments in the following code - I'll leave it to you to eliminate them...

12. check(a); //check if any new process has arrived
Nothing inherently wrong here - however, above I was commenting on the function check() - do you see the problem right here? Hint: there are more calls to check() later...

13. struct data aa[n];
Wrong aarray declaration. See item 8 above.

14. cin>>aa[i].pname;
Honestly, I don't know if this would work as intended. I don't know because I'd never use iostream to read a char array. It's unclear what happens if you accidentally enter a longer string than you reserved a buffer for, and it's unclear what happens if you enter a shorter string too - although I suspect that might work alright. In fact, I'm not even sure it would work at all, because, technically, aa[i].pname is a pointer, and I'm not at all sure if the information that this is a fixed size character array is even passed to cin! (it probably is, but I'll not bother trying, because there are better ways to read strings!)

15. The sort loop is missing a final closing bracket.

This is just what I get from looking at the code, without trying to figure out what it does, or is supposed to do (which likely isn't the same thing right now)

If you need any further advice on one or more of the issues I pointed out, let me know.