Sorting Data

[The Conqueror]

Define a structure called c ricket the will describe the following information -
i) Player's name, ii) Country name, iii) Batting average.
Write a program that will store information of 25 players using this structure, also arrange and display names of these players in descending order with respect to batting average.


Here is my code :

#include <stdio.h>
#include <conio.h>
int main()

{
    
    struct cricket
    {
           char name[25];
           char country[25];
           int bestscore;
           float average;
           };
    struct cricket s[25],t;
    int i; int j;
    int n=25;

for(i=0; i<n ; i++)
{
         printf("\n Enter Player's Name");
         scanf("%s", s[i].name);
         printf("\n Enter Country");
         scanf("%s", s[i].country);
         printf("\n Enter Best Score");
         scanf("%d", &s[i].bestscore);
         printf("\n Enter Batting Average");
         scanf("%f", &s[i].average);
         }
         
for(i=0; i<n-1; i++)
{
         for(j=i+1; j<n; j++)
         {
                    if(s[i].average<s[j].average)
                    {
                                                 t=s[i];
                                                 s[i]=s[j];
                                                 s[j]=t;
                                                 }
                                                 }
                                                 }
printf("Names of players in DESCENDING order w.r.t batting averages \n\n");

for(i=0; i<n; i++)
{
         printf("Name of Player : %s \t", s[i].name);
         printf("Batting Average : %f \n", s[i].average);
         }
getch();
return 0;
}

This is my code. Is there a way I can optimize it further? I lost 4 marks in class test because I did not use "gets()" function for string input and string.h library function. But it does work fine without both of them. I've read on forums that "gets" function is dangerous. So I'm confused what to do? Will using this program affect my grade in the University exams? (University of Mumbai)?

 

[harshilsharma63]

Yes, using gets() can cause buffer overflow issues. it has been replaced with _gets(). Use visual studio for the latest and most up to date c++ standards.

Optimizations:

1. change the structure definition to:

struct cricket
{
    int bestscore;
    float average;
    string name;
    string country;
};

declaring structure members in increasing order of their size minimizes the time taken to reach all members in memory.

2. instead of writing:

 int i; int j;
int j=25;

you should write:

 int i=0, j=0, n=25;

this reduces the number of lines in code, improves elegance, and prevents unexpected conditions to arrive due to uninitialized variables.

3. using n++; instead of ++n; has no difference when used alone, but when post increment is not needed, use pre increment; no performance increase, just a good habit IMO as post increments use more memory than pre increment operator.

4. the sorting algorithm can always be improved.

5. Last but most important, USE COMMENTS!


check this link: Optimizing C++/Print Version - Wikibooks, open books for an open world

 

[rijinpk1]

avoid scanf() as it is not possible to store middle and last name for this particular example. use gets(). Also you can use getchar() to input each character.

 

[The Conqueror]

avoid scanf() as it is not possible to store middle and last name for this particular example. use gets(). Also you can use getchar() to input each character.

I tried to use gets() but for some unknown reason, it doesn't ask for player's name from second iteration onwards.

Yes, using gets() can cause buffer overflow issues. it has been replaced with _gets(). Use visual studio for the latest and most up to date c++ standards.

Optimizations:

1. change the structure definition to:

struct cricket
{
    int bestscore;
    float average;
    string name;
    string country;
};

declaring structure members in increasing order of their size minimizes the time taken to reach all members in memory.

2. instead of writing:

 int i; int j;
int j=25;

you should write:

 int i=0, j=0, n=25;

this reduces the number of lines in code, improves elegance, and prevents unexpected conditions to arrive due to uninitialized variables.

3. using n++; instead of ++n; has no difference when used alone, but when post increment is not needed, use pre increment; no performance increase, just a good habit IMO as post increments use more memory than pre increment operator.

4. the sorting algorithm can always be improved.

5. Last but most important, USE COMMENTS!


check this link: Optimizing C++/Print Version - Wikibooks, open books for an open world

Thanks a lot That really helped.

 

[hotshot05]

I tried to use gets() but for some unknown reason, it doesn't ask for player's name from second iteration onwards.


Thanks a lot That really helped.

You have to flush the keyboard buffer after every iteration using the statement fflush(stdin)

If you use only scanf(), the program will read the characters till it encounters a space. Everything after the space will be ignored.

 

[vickybat]

@ The Conquerer

You can definitely improve in sorting algorithms. What you're currently employing is the simple bubble sort.
Its the easiest, but the least efficient. For each comparison, you do a swap. So n-1 comparisons will have a maximum of n-1 swaps, if everything is in the ascending order.

I suggest you try out the selection sort and quick sort algorithms.

Here's the selection sort one. It just requires a slight modification to your sorting code:

for(i=0; i<n-1; i++)
{
 int max = i;
         for(j=i+1; j<n; j++)
         {
          
             if(s[j].average > s[max].average)
                max = j;
                   }
                   t=s[i];
                   s[i]=s[max];
                   s[max]=t;
                                                 
                                                 
      }

Here, you define a max variable which is initially the first element in the array. Its assumed to have the maximum value ( as you're sorting in a descending order).
Its compared with rest of the array and the maximum value in the array is swapped with the original at the end of the comparison. So for n-1 comparisons, you just do one swap as compared to n-1 swaps in bubble sort. So that's efficient.

The quick sort is even more efficient. Try that out yourself and post your code here mate.

 

[harshilsharma63]

Or create a linked list of structure instead of array and use insertion sort.

 

[The Conqueror]

@ The Conquerer

You can definitely improve in sorting algorithms. What you're currently employing is the simple bubble sort.
Its the easiest, but the least efficient. For each comparison, you do a swap. So n-1 comparisons will have a maximum of n-1 swaps, if everything is in the ascending order.

I suggest you try out the selection sort and quick sort algorithms.

Here's the selection sort one. It just requires a slight modification to your sorting code:

for(i=0; i<n-1; i++)
{
 int max = i;
         for(j=i+1; j<n; j++)
         {
          
             if(s[j].average > s[max].average)
                max = j;
                   }
                   t=s[i];
                   s[i]=s[max];
                   s[max]=t;
                                                 
                                                 
      }

Here, you define a max variable which is initially the first element in the array. Its assumed to have the maximum value ( as you're sorting in a descending order).
Its compared with rest of the array and the maximum value in the array is swapped with the original at the end of the comparison. So for n-1 comparisons, you just do one swap as compared to n-1 swaps in bubble sort. So that's efficient.

The quick sort is even more efficient. Try that out yourself and post your code here mate.

Or create a linked list of structure instead of array and use insertion sort.

Thank you for your suggestions. As a matter of fact, a very similar problem was asked yesterday for sorting employee data. I used bubble-sort because that's the only algorithm in our syllabus, and the ones you mentioned are beyond the scope of the university syllabus. And my university is notorious for deducting marks on baseless reasons. However, I'll certainly try to solve the same problem using different algorithms and linked list