Using bucket sort to rank students based on their marks

Question

Problem:

n students have marks and names. Rank students based on their marks and print their names.

---

#include <stdio.h>
#include <assert.h>

struct student
{
    int marks;
    char name[50];
};

struct listnode
{
    struct student * data;
    struct listnode *next;
};

addtofrontoflist(struct listnode *listhead,struct student *node)
{
    //make a block of listnode.
    struct listnode bucketnode;
    bucketnode.data=node;

    struct listnode *tmp;
    tmp=listhead->next;
    listhead->next=&bucketnode;
    bucketnode.next=tmp;
}

bucket_sort(struct student array[],int n)
{
    struct listnode *buckets[n]; 
    int i,j;
    struct listnode *temp;
    for(i=0;i<n;i++)
    {
            buckets[i]=malloc(sizeof(struct listnode));
            assert(buckets[i]!=NULL);
    }

    for(i=0;i<n;i++)
    {
            addtofrontoflist(buckets[array[i].marks],&array[i]);
    }

    for(i=0;i<n;i++)
    {
            if(buckets[i]==NULL) continue;
            for(temp=buckets[i];temp!=NULL;temp++)
            printf("%d %s\n",temp->data->marks,temp->data->name);
    }
}

main()
{
    struct student array[5]={"1,one","2,two","2,two","4,four","1,one"};
    bucket_sort(array,5);
}

Answer 1

• Since your functions have no explicit return types (and surely you've compiled this), you must be implementing this under an older C standard that allowed for implicit return types.

  Regardless, I would recommend adding explicit return types to your functions:

  • main() should return an int as it is supposed to return an integer value based on the program's execution outcome (either 0 for success or 1 for failure). However, if it will always execute successfully, then you can omit the return 0 at the end.

  • The other functions should return void as they're not already returning anything.

• You may typedef your structs to avoid having to type struct elsewhere.

• You're inconsistently using snake_case naming and all-lowercase. The latter is not very readable as compound words should be separated. Since you're already using snake_case, just use that for all of your variable and function naming here.

• marks should probably just be mark as it's just a single variable. But if a student is supposed to have more than one, then marks should be an array. Also, must this be an integer type, or can a student have a decimal value as a mark? This would of course depend on how you want to implement this grading system.

• The variable j declared in bucket_sort() is unused, so it should be removed. You should have your compiler warnings up high so that you'll be told when this occurs.

• You should add whitespace between operators for readability. Keeping lines shorter would not be of much benefit as you should try not to have so much code on one line.

  For instance, one of your for loops:

  for(i=0;i<n;i++)
  

  would look like this:

  for (i = 0; i < n; i++)
  

  And your array:

  struct student array[5]={"1,one","2,two","2,two","4,four","1,one"};
  

  should have its elements separated:

  struct student array[5] = { "1,one", "2,two", "2,two", "4,four", "1,one" };
  

• I'd recommend having a macro for the number of students:

  #define NUM_STUDENTS 5
  

  You can use this macro for the array size and anything else, and in case you'll ever need to change this number, this will be done in just one place.

  In addition, bucket_sort() will no longer need to take a size.

  student array[NUM_STUDENTS] = { "1,one","2,two","2,two","4,four","1,one" };
  bucket_sort(array);
  

Answer 2

First, make sure the code compiles. You should include stdlib.h for malloc. Depending on which compiler and libraries you use, this may still work on your system but always include the proper headers for the api's you're using.

Then your initialization of the student array in main is wrong. Your compiler should have given you a warning about this too. If not, pump up the warning level! You probably want this instead:

struct student array[5] = {{1, "one"}, {2, "two"}, {2, "two"}, {4, "four"}, {1, "one"}};

Also make sure you specify the return type of each function. If you don't return a value, make the return type void. If you don't specify the function default to return int, and your compiler should give you a warning that you exit without returning a value. (Pay attention to warnings!)

Another thing with functions, try to name them properly and make sure they do what the function name says. Your function bucket_sort does not do what advertised. It first converts an array of student to buckets, then tries to sort it and then prints the result. A name like print_students_sorted_by_mark() would probably be better. And better yet, you might want to split this into three functions.

The sorting also looks broken, but I have not looked to closely into that now.

In general you may want to pay more attention to naming. You use both listnode and bucketnode/bucket to signify the same thing. Cleaning up naming and keeping it consistent will mean a lot to others reading your code, especially in bigger projects.