Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Aim is to provide total head movement in different disk scheduling algorithms namely FCFS, SSTF, LOOK, C-LOOK, SCAN, C-SCAN.

#include <iostream>
#include <cstdlib>
#include <conio.h>
#include <math.h>

using namespace std;

int compare (const void * a, const void * b)
{
  if ( *(int*)a <  *(int*)b ) return -1;
  if ( *(int*)a == *(int*)b ) return 0;
  if ( *(int*)a >  *(int*)b ) return 1;
}
void shift(int a[],const int size)
{
    for(int i=1;i<=size;i++)
        a[i-1]=a[i];
}
class disk
{
private :
    int request[101];
int sorted_request[101];
int number_of_request;
int max;
int direction;
public :
disk()
{
    cout<<"\nEnter maximum disk limit : ";
    cin>>max;
}

void receive_request()
{
    enter_request_number:
    cout<<"\nEnter number of requests (max. 100): ";
    cin>>number_of_request;
    if(number_of_request>100)
        goto enter_request_number;

    current_location:
    cout<<"\nEnter current location : ";
    cin>>request[0];
    sorted_request[0]=request[0];
    if(request[0]>max||request[0]<0)
        goto current_location;

    current_direction:
    cout<<"\nEnter current direction(0:left / 1:right) : ";
    cin>>direction;
    if(direction!=0&&direction!=1)
        goto current_direction;


    for(int i=1;i<=number_of_request;i++)
    {
        cout<<"\nEnter request number "<<i<<" : ";
        cin>>request[i];
        sorted_request[i]=request[i];
        if(request[i]>max||request[i]<0)
        {
            cout<<"\nInvalid request !! Enter again!!";
            i--;
        }
    }
    qsort(sorted_request+1,number_of_request,sizeof(int),compare);

}

int fcfs()
{
    int head_movement=0;
    for(int i=1;i<=number_of_request;i++)
        head_movement+=abs(request[i]-request[i-1]);
    return head_movement;
}
int sstf()
{
    int head_movement=0,flag=0,nor=number_of_request;
    int request[101];
    request[0]=sorted_request[0];
    for(int i=1;i<=number_of_request;i++)
    {
        if(sorted_request[i]>sorted_request[0]&&flag==0)
            flag=i;
        request[i]=sorted_request[i];
    }
    while(nor)
    {
        if(flag==0)
        {
            head_movement+=request[0]-request[nor];
            request[0]=request[nor];
        }
        else if(flag==1)
        {
            head_movement+=abs(request[nor]-request[0]);
            break;
        }
        else if((request[flag]-request[0])>(request[0]-request[flag-1]))
        {
            head_movement+=request[0]-request[flag-1];
            request[0]=request[flag-1];
            flag--;
            shift(request+flag,nor-flag);
        }
        else 
        {
            head_movement+=request[flag]-request[0];
            request[0]=request[flag];
            shift(request+flag,nor-flag);
        }
        nor--;
    }
    return head_movement;
}
int SCAN()
{
    int head_movement=0,flag=0;

    for(int i=1;i<=number_of_request;i++)
        if(sorted_request[i]>sorted_request[0]&&flag==0)
            flag=i;

    if(direction==1)
    {
        if(flag==1)
            head_movement+=sorted_request[number_of_request]-sorted_request[0];

        else 
        {
            head_movement+=max-sorted_request[0];
            head_movement+=max-sorted_request[1];
        }   
    }
    else
    {
        if(flag==0)
            head_movement+=abs(sorted_request[number_of_request]-sorted_request[0]);

        else 
        {
            head_movement+=sorted_request[0];
            head_movement+=sorted_request[number_of_request];
        }
    }
    return head_movement;
}
int CSCAN()
{
    int head_movement=0,flag=0;

    for(int i=1;i<=number_of_request;i++)
        if(sorted_request[i]>sorted_request[0]&&flag==0)
            flag=i;

    if(flag==1)
        head_movement+=sorted_request[number_of_request]-sorted_request[0];
    else 
    {
        head_movement+=max-sorted_request[0];
        head_movement+=max;
        head_movement+=max-sorted_request[flag-1];
    }   

    return head_movement;
}


int LOOK()
{
    int head_movement=0,flag=0;

    for(int i=1;i<=number_of_request;i++)
        if(sorted_request[i]>sorted_request[0]&&flag==0)
            flag=i;

    if(direction==1)
    {
        if(flag==1)
            head_movement+=sorted_request[number_of_request]-sorted_request[0];

        else 
        {
            head_movement+=sorted_request[number_of_request]-sorted_request[0];
            head_movement+=sorted_request[number_of_request]-sorted_request[1];
        }   
    }
    else
    {
        if(flag==0)
            head_movement+=abs(sorted_request[number_of_request]-sorted_request[0]);

        else 
        {
            head_movement+=sorted_request[1];
            head_movement+=sorted_request[number_of_request]-sorted_request[1];
        }
    }
    return head_movement;
}
int CLOOK()
{
    int head_movement=0,flag=0;

    for(int i=1;i<=number_of_request;i++)
        if(sorted_request[i]>sorted_request[0]&&flag==0)
            flag=i;

    if(flag==1)
        head_movement+=sorted_request[number_of_request]-sorted_request[0];
    else 
    {
        head_movement+=sorted_request[number_of_request]-sorted_request[0];
        head_movement+=sorted_request[number_of_request]-sorted_request[1];
        head_movement+=sorted_request[flag-1]-sorted_request[1];
    }   

    return head_movement;
}
~disk(){}
};

int main()
{
    disk hdd;
    hdd.receive_request();
    cout<<"Total head movement in ";
    cout<<"FCFS is "<<hdd.fcfs()<<endl;
    cout<<"Total head movement in ";
    cout<<"SSTF is "<<hdd.sstf()<<endl;
    cout<<"Total head movement in ";
    cout<<"SCAN is "<<hdd.SCAN()<<endl;
    cout<<"Total head movement in ";
    cout<<"CSCAN is "<<hdd.CSCAN()<<endl;
    cout<<"Total head movement in ";
    cout<<"LOOK is "<<hdd.LOOK()<<endl;
    cout<<"Total head movement in ";
    cout<<"CLOOK is "<<hdd.CLOOK()<<endl;
    _getche();
}  
share|improve this question

2 Answers 2

up vote 12 down vote accepted

You are using C++, but overall, your code looks like C. There are numerous ways to improve your code readability by replacing C standard features by C++ ones. I will give some examples.

using namespace std;

Wirting using namespace std; is generally considered bad practice, especially when written in the global namesapce: it leads to potential name clashes and namespace pollution. You should just add std:: before every function/calss/variable from the standard library.

Headers

Several remarkes here:

  • <math.h> is a C header, in C++, you should include <cmath> instead, which is its C++ equivalent.
  • <conio.h> is an old non-portable header. For input/output operations, you should generally use <iostream> and stick with it.
  • Also, it's good practice to organize your include directives in alphabetical ordering. That helps to check whether some header has already been included or not, and to avoid including twice some header.

Standard library collections

Instead of using old C arrays and pointers, you can use standard library containers. For example, a fixed sized array int arr[30] can be replaced by the equivalent std::array<int, 30> in C++11 (this container is not directly available in the older versions of the standard).

Standard library algorithms

Instead of the old qsort, which requires to take a function pointer with void* pointers and do some ugly casts, you should use std::sort in the standard header <algorithm>. It should be at least as fast (if not faster) and easier to read and write. You can replace this line:

qsort(sorted_request+1,number_of_request,sizeof(int),compare);

by this one:

std::sort(std::begin(sorted_request), std::end(sorted_request));

Miscellaneous notes

I can't cover all your code because it would have to be refactored to use more standard library features beforehand, but here are some notes:

  • Upper case names should only be used for preprocessor macros. Therefore, your functions CSCAN and LOOK (and the other ones) should be named cscan and look instead.
  • I'm not at ease with using std::cin in the constructor of disk. Such user interaction should not appear at construction, but be handled later by the client code of the class.
  • if(direction=1) is probably a bug, it should be if(direction==1), using proper compiler flags (-Wall, -Wextra, etc...) should tell you when you use an assignment instead of a comparison in if.
  • There is a typo in recieve_request: it should be receive_request.

Overall, you should really consider looking at the C++ standard library and using its classes and algorithms to replace many things in your code.

share|improve this answer
    
It (std::sort) should be as fast (as qsort). Actually, in a lot of cases, it should be significantly faster, as the comparison can generally be inlined - whereas it usually cannot be for qsort. –  Yuushi Apr 27 at 14:26
    
@Yuushi I'll add "at least" :p –  Morwenn Apr 27 at 14:29
    
@Morwenn: Good call--in fact, in this case (comparing ints) std::sort will most likely be noticeably faster than qsort. –  Jerry Coffin Apr 27 at 17:50
    
@Morwenn Can you please explain how to use this line : std::sort(std::begin(sorted_request), std::end(sorted_request)); when I do not want to start form begning or go upto the end –  vidit jain Apr 27 at 18:33
2  
@viditjain It's like pointer arithmetic. Thus, if you want to sort from 3 to 10 do: std::sort(std::begin(sorted_request) + 3, std::begin(sorted_request) + 10); –  black Apr 27 at 19:00

Morwenn has already given feedback I'd consider quite good, so I'll restrict myself to areas he hasn't already covered.

First, you have some code I think could be structured quite a bit more nicely. Contrary to Morwenn's comments, some parts of your code look more like ancient Fortran or BASIC rather than C. For example:

current_direction:
cout<<"\nEnter current direction(0:left / 1:right) : ";
cin>>direction;
if(direction!=0&&direction!=1)
    goto current_direction;

Even in C, you'd normally prefer a do/while loop to a goto for this:

do { 
    cout<<"\nEnter current direction(0:left / 1:right) : ";
    cin>>direction;
} while (direction != 0 && direction != 1);

This particular loop also seems to be asking for data that it seems like it has essentially no business asking for in the first place. I'd think if you're going to simulate a disk drive, the inputs should simply be disk locations of reads/writes, and possibly an initial head location (with some sensible default, such as track 0). Pretty much everything else should be results from the simulation, not inputs to it.

At a somewhat higher level, this doesn't strike me as a particularly good type of program to make interactive. To produce results that mean much of anything it almost certainly needs to process quite a bit of input. Personally, I can't imagine interactively entering the data for even one meaningful run, not to mention (for example) two or three runs to compare results for different usage patterns.

At least as a user, I'm quite certain I'd strongly prefer a program that let me enter data into a file, and just specify the name of that file on the command line to get a set of results. This gives me much better dependability and repeatability. Just for example, if I get strange results from a particular set of inputs, I want to be able to re-examine those inputs, assure they were what I intended, re-run the simulation if needed, and so on.

I hope it doesn't sound condescending or nasty (because I honestly don't intend it that way), but this is a large part of what separates toys from tools. As it stands right now, your program is basically a toy. You can run it a few times to prove that it produces reasonable output for a small amount of input, but I can't imagine that anybody would actually want to use it on a regular basis.

Using it is too labor intensive, and requires nearly perfect attention to detail on the part of the user. It's all too easy for the user to slip up and enter the same data twice, skip data they intended to enter, etc. Worse, when a user gets a particular set of results it's next to impossible for them to be sure what input produced that results. Simulations are typically used for lots of "what if" kinds of situations--in a typical case, a user wants to know what happens if one specific detail in one place changes. With a program like this, he has to re-enter all the data just to make that one minor change in one place.

So, while it's important to clean up the internals as well, I think the biggest and most important change that's needed is to define some simple file format for inputs, and at least allow the user to specify a file as the input instead of forcing them to enter their data interactively. The file format doesn't need to be complex at all, but (if you intend this to be put to real use) you might consider some format that's at least somewhat standardized (and there are lots of those to choose from).

share|improve this answer
    
why is using goto considered a bad practice?? –  vidit jain Jul 28 at 18:21
    
@viditjain: Primarily because using goto tends to lead to code with control structures that are difficult to understand or reason about. With something like do {...} while (x);, I can expect x to be false immediately after the loop exits. –  Jerry Coffin Jul 28 at 20:27

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.