0
\$\begingroup\$

I have developed a program that implements a Priority Scheduling algorithm (Priority Scheduling). The total number of processes is read directly from the standard input, each process specifying the arrival time and the execution time, respectively its priority. I calculated the average response time, respectively the average waiting time.

Is my implementation optimal? What can I change?

One known problem I have is that if I enter a larger number of processes, then not every process will be displayed under the ID column.

Code:

#include<stdio.h>
struct process
{
    int WT,AT,BT,TAT,PT;
};
struct process a[10];
int main()
{
    int n,temp[10],t,count=0,short_p;
    float total_WT=0,total_TAT=0,Avg_WT,Avg_TAT;
    printf("Enter the number of processes:\n");
if (scanf("%d", &n) == 1) {

    printf("Enter arrival time, execution time and priority\n");
    printf("Arrival Execution Priority\n");
} else {
        printf("Fail\n");
    return 1;
    }

       for(int i=0;i<n;i++)
    {
if(scanf("%d",&a[i].AT)==1){
    } else{
        printf("Fail\n");
        return 1;
        }

        if(scanf("%d",&a[i].BT)==1){
        temp[i]=a[i].BT;
    }

        else{
        printf("Fail\n");
        return 1;
        }

        if(scanf("%d",&a[i].PT)==1){
    }else{
        printf("Fail\n");
        return 1;
        }

    }
a[9].PT=10000;
for(t=0;count!=n;t++)
    {
        short_p=9;
        for(int i=0;i<n;i++)
        {
            if(a[short_p].PT>a[i].PT && a[i].AT<=t && a[i].BT>0)
            {
                short_p=i;
            }
        }
a[short_p].BT=a[short_p].BT-1;
            count++;
            a[short_p].WT=t+1-a[short_p].AT-temp[short_p];
            a[short_p].TAT=t+1-a[short_p].AT;
            total_WT=total_WT+a[short_p].WT;
            total_TAT=total_TAT+a[short_p].TAT;
    Avg_WT=total_WT/n;
    Avg_TAT=total_TAT/n;
    printf("ID WT TAT\n");
    for(int i=0;i<n;i++)
    {
        printf("%d %d\t%d\n",i+1,a[i].WT,a[i].TAT);
    }
    printf("The average waiting time for the process is: %f\n",Avg_WT);
    printf("The average return time of the process is: %f\n",Avg_TAT);
    return 0;
}
```
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Variable Names

In 6 months time the one and two letter variable names won't mean anything to you or anyone else that needs to maintain the code. Use descriptive variable names and make the code as self documenting as possible.

Declare the Variables as Needed

In the original version of C back in the 1970s and 1980s variables had to be declared at the top of the function. That is no longer the case, and a recommended programming practice to declare the variable as needed. In C the language doesn't provide a default initialization of the variable so variables should be initialized as part of the declaration. For readability and maintainability each variable should be declared and initialized on its own line. See the declarations in the example at the bottom.

Complexity

The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

There are at least 3 possible functions in main().

  • Get the count of processes and check for errors
  • Get get the process related values
  • Calculate and Print Results

The complexity of the code is added to by the empty then clauses in the if statements, they can be simplified by reversing the logic in the expression.

        if (scanf("%d", &a[i].AT) != 1) {
            printf("Fail\n");
            return 1;
        }

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation. The array of processes a should be defined in main and passed by reference into the functions that need it.

Magic Numbers

There are Magic Numbers in the main() function (10, 0, 1, ), it might be better to create symbolic constants for them to make the code more readable and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintenance easier. Examples of confusing code:

    a[9].PT = 10000;    // What is 10,000 and why are you assigning it to a[9].PT?

    return 0;
    return 1;

    struct process [10]

For return 1 or return 0 include stdlib.h and use the common symbolic constants EXIT_FAILURE and EXIT_SUCCESS.

For array sizes define a symbolic constant using :

#define MAX_PROCESSES 10

This allows you to change the maximum number of processes with a one line edit, if needed you can use it in for loops as well

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.

Refactored Code Using Above Suggestions

This probably not how I would have solved the problem, but I used your code as much as possible to show what I mean. Rather than used a fixed size array I probably would have used calloc() to allocate dynamic arrays at runtime, that is more user friendly then small fixed size arrays. Everything I did change was to reduce the complexity of the code and make it more readable and maintainable. I did not test my refactoring to see if it worked, it is only a collection of the suggestions.

#include<stdio.h>
#include<stdlib.h>

#define MAX_PROCESSES 10
struct process
{
    int WT;
    int AT;
    int BT;
    int TAT;
    int PT;
};

int get_process_count()
{
    int process_count = 0;

    printf("Enter the number of processes:\n");
    if (scanf("%d", &process_count) != 1) {
        return 0;
    }

    return process_count;
}

int get_process_data(int process_count, struct process a[MAX_PROCESSES])
{
    printf("Enter arrival time, execution time and priority\n");
    printf("Arrival Execution Priority\n");

    for (int i = 0; i < process_count; i++)
    {
        if (scanf("%d", &a[i].AT) != 1) {
            printf("Fail\n");
            return 1;
        }

        if (scanf("%d", &a[i].BT) != 1) {
            printf("Fail\n");
            return 1;
        }

        if (scanf("%d", &a[i].PT) != 1) {
            printf("Fail\n");
            return 1;
        }
    }

    return 0;
}

void process_and_report_results(int process_count, struct process a[MAX_PROCESSES])
{
    int t = 0;
    int count = 0;
    int short_p = 0;
    float total_WT = 0.0;
    float total_TAT = 0.0;
    float Avg_WT, Avg_TAT;

    a[9].PT = 10000;
    for (t = 0; count != process_count; t++)
    {
        short_p = 9;
        for (int i = 0; i < process_count; i++)
        {
            if (a[short_p].PT > a[i].PT && a[i].AT <= t && a[i].BT > 0)
            {
                short_p = i;
            }
        }

        int temp = a[t].BT;
        a[short_p].BT = a[short_p].BT - 1;
        count++;
        a[short_p].WT = t + 1 - a[short_p].AT - temp;
        a[short_p].TAT = t + 1 - a[short_p].AT;
        total_WT = total_WT + a[short_p].WT;
        total_TAT = total_TAT + a[short_p].TAT;
        Avg_WT = total_WT / process_count;
        Avg_TAT = total_TAT / process_count;

        printf("ID WT TAT\n");
        for (int i = 0; i < process_count; i++)
        {
            printf("%d %d\t%d\n", i + 1, a[i].WT, a[i].TAT);
        }
        printf("The average waiting time for the process is: %f\n", Avg_WT);
        printf("The average return time of the process is: %f\n", Avg_TAT);
    }
}

int main()
{
    struct process a[MAX_PROCESSES];
    int process_count = 0;

    process_count = get_process_count();
    if (process_count < 1)
    {
        return EXIT_FAILURE;
    }

    if (get_process_data(process_count, a) < 1)
    {
        return EXIT_FAILURE;
    }

    process_and_report_results(process_count, a);

    return EXIT_SUCCESS;
}

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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