Sum and bubble-sort student scores

Question

I am a beginner in C language.

I studied all night and did my assignment.

But I wonder if there is a more efficient way.

The original structure is this.

typedef struct student {
    int id;
    int prob_1;
    int prob_2;
    int prob_3;
    int total_score;
    char grade;
}Student

I used typedef to change it to Student.

And I'll show you the contents of the function.

void cal_total_score(Student* s) {
    /*
    Problem (A):
    Implement the function "cal_total_score".
    - In each student, total_score must be the sum of prob_1, prob_2, and prob_3.
    - The codes outside the function must not be changed.
    */
    int i;
    for (i = 0; i < NUMSTUDENT; i++) {
        s[i].total_score = s[i].prob_1 + s[i].prob_2 + s[i].prob_3;
    }
}

void student_bb_sort(Student* s) {
    /*
    Problem (B):
    Implement the function "student_bb_sort".
    - The student array "s" must be sorted according to total_score.
    - Bubble sort algorithm should be used for the sorting.
    - The codes outside the function must not be changed.
    */
    Student tmp;
    int i, j;

    for (i = 0; i < NUMSTUDENT - 1; i++) {
        for (j = 0; j < NUMSTUDENT - 1 - i; j++) {
            if (s[j].total_score > s[j + 1].total_score) {
                tmp = s[j];
                s[j] = s[j + 1];
                s[j + 1] = tmp;
            }
        }
    }
}

Please let me know if there are any more wrong or dangerous phrases or efficient ways.

---

This is a given main function in advance, so read it if you need it.

typedef struct student {
    int id;
    int prob_1;
    int prob_2;
    int prob_3;
    int total_score;
    char grade;
}Student;

void cal_total_score(Student* s);
void student_bb_sort(Student* s);

int main() {
    Student s[NUMSTUDENT];
    int i;

    for (i = 0; i < NUMSTUDENT; i++) {
        s[i].id = i;
        s[i].prob_1 = rand() % 30 + 1;
        s[i].prob_2 = rand() % 30 + 1;
        s[i].prob_3 = rand() % 40 + 1;
        printf("Student %d\nP1: %d\tP2: %d\tP3: %d\n", s[i].id, s[i].prob_1, s[i].prob_2, s[i].prob_3);
    }

    for (i = 0; i < NUMSTUDENT; i++) {
        cal_total_score(&s[i]);
        printf("Student %d\tScore: %d\n", s[i].id, s[i].total_score);
    }

    student_bb_sort(s);
    printf("\nAfter Bubble Sort\n");

    for (i = 0; i < NUMSTUDENT; i++) {
        if ((float)i < (float)NUMSTUDENT / 3) {
            s[i].grade = 'C';
        }
        else if ((float)i < (float)NUMSTUDENT / 3 * 2 && (float)i >= (float)NUMSTUDENT / 3) {
            s[i].grade = 'B';
        }
        else {
            s[i].grade = 'A';
        }
        printf("Student %d\tScore: %d\tGrade: %c\n", s[i].id, s[i].total_score, s[i].grade);
    }

Answer 1

But I wonder if there is a more efficient way.

Instead of bubble-sort, use qsort(). Yet I expect OP's assignment may oblige bubble-sort.

#include <stdlib.h>

int student_compar(const void *p1, const void *p2) {
  const Student *s1 = (const Student *) p1;
  const Student *s2 = (const Student *) p2;
  return (s1->total_score > s2->total_score) - (s1->total_score < s2->total_score);
}

...
// student_bb_sort(s);
qsort(s, NUMSTUDENT, sizeof s[0], student_cmp);

OP's code does not look like a proper bubble_sort as it lacks the quick solution when the array is already sorted. Perhaps as

#include <stdbool.h>

void student_bb_sort_alt(size_t n, Student* s) {
  bool sorted;
  do {
    sorted = true;
    for (size_t i = 1; i < n; i++) {
      if (s[i - 1].total_score > s[i].total_score) {
        Student tmp = s[i];
        s[i] = s[i - 1];
        s[i - 1] = tmp;
        sorted = false;
      }
    }
  } while (!sorted);
}

Assign all the members, Fold loops together

Calculate total score right after the .prob_3 assignment. A good practice it to avoid leaving members unassigned for long. As the .grade is not know until much later, consider filling now with U for unknown. This allows a common student_display() function to be called right away without unambiguous results, even before the sort.

    s[i].prob_3 = rand() % 40 + 1;

    // Add
    s[i].total_score = s[i].prob_1 + s[i].prob_2 + s[i].prob_3;
    s[i].grade = 'U';

Avoid float for common FP math

Use double for typical floating point (FP) math. Use float in select cases when the size/(and sometimes speed) is important and not precision/range.

Definitely drop the && (float)i >= (float)NUMSTUDENT / 3. The test is redundant due to the prior if() and floating point math simple does things on edge cases that may cause this test to fail when you expect it to be true.

Use integer math when able

Use named thresholds. It self-documents code.

// for (i = 0; i < NUMSTUDENT; i++) {
//     if ((float)i < (float)NUMSTUDENT / 3) {
//         s[i].grade = 'C';
//     }
//     else if ((float)i < (float)NUMSTUDENT / 3 * 2 && (float)i >= (float)NUMSTUDENT / 3) {
//         s[i].grade = 'B';
//     }
//     else {
//         s[i].grade = 'A';
//     }
//     printf("Student %d\tScore: %d\tGrade: %c\n", s[i].id, s[i].total_score, s[i].grade);
// }

int B_min = (NUMSTUDENT + 2)/ 3; // + 2 to function like (float)i < (float)NUMSTUDENT / 3.
int A_min = (NUMSTUDENT*2 + 2)/ 3;
for (i = 0; i < NUMSTUDENT; i++) {
    if (i < B_min) {
        s[i].grade = 'C';
    }
    else if (i < A_min) {
        s[i].grade = 'B';
    }
    else {
        s[i].grade = 'A';
    }
    printf("Student %d\tScore: %d\tGrade: %c\n", s[i].id, s[i].total_score, s[i].grade);
}

Advanced: Array indexing and sizing

In select cases, int may be of insufficient range. size_t is right sized for array indexing and object sizing. Note that size_t is some unsigned type.

//int i, j;
size_t i, j;

for (i = 1; i < NUMSTUDENT; i++) {

Bug!!

OP's grading may result in 2 students with the same score getting different grades.

Imagine all sorted students in the 30% to 35% ordering had the same total. Should not everyone get the same grade?

Instead of the first third getting a C. Use the 33% ordered student's score as a threshold.

int B_min = s[(NUMSTUDENT + 2)/ 3].total_score;
int A_min = s[(NUMSTUDENT*2 + 2)/ 3].total_score;
for (i = 0; i < NUMSTUDENT; i++) {
    if (s[i].total_score < B_min) {
        s[i].grade = 'C';
    }
    else if (s[i].total_score < A_min) {
        s[i].grade = 'B';
    }
    else {
        s[i].grade = 'A';
    }
    printf("Student %d\tScore: %d\tGrade: %c\n", s[i].id, s[i].total_score, s[i].grade);
}

Answer 2

Advice 1

typedef struct student {
    int id;
    int prob_1;
    int prob_2;
    int prob_3;
    int total_score;
    char grade;
}Student;

Since prob_1, prob_2, prob_3 and total_score are supposed to be positive integers, I suggest you change their types to unsigned int. (Don't forget to include <stdlib.h>.) Also, I personally would change the type of id to size_t, too. Finally, you can typedef the student to student with the prefix s and not S. This buys you the possibility to declare the student data in both flavors:

struct student s1;
student s2;

Advice 2

Don't use magic numbers. You could write:

#define NUM_STUDENTS 3
#define MAX_PROBLEM_1_SCORE 30
#define MAX_PROBLEM_2_SCORE 30
#define MAX_PROBLEM_3_SCORE 40

#define MAX_SCORE (MAX_PROBLEM_1_SCORE + MAX_PROBLEM_2_SCORE + MAX_PROBLEM_3_SCORE)

Advice 3

A common pattern for accepting arrays via function arguments is to pass the pointer to the leftmost array component and the lenght of the array:

void cal_total_score(student* s, size_t length);
void student_bb_sort(student* s, size_t length);

Advice 4

Seed you random:

srand(time(NULL));

Or otherwise, you will produce exactly the same random numbers. (Don't forget to include <time.h> for time(NULL).)

Advice 5

for (i = 0; i < NUMSTUDENT; i++) {
    s[i].id = i;
    s[i].prob_1 = rand() % 30 + 1;
    s[i].prob_2 = rand() % 30 + 1;
    s[i].prob_3 = rand() % 40 + 1;
    printf("Student %d\nP1: %d\tP2: %d\tP3: %d\n", s[i].id, s[i].prob_1, s[i].prob_2, s[i].prob_3);
}

for (i = 0; i < NUMSTUDENT; i++) {
    cal_total_score(&s[i]);
    printf("Student %d\tScore: %d\n", s[i].id, s[i].total_score);
}

Why not combine all the output into one for loop?

Advice 6

You could optimize your bubble sort to quit when a pass over the student array does not lead to swapping the students in the array. See below:

Alternative implementation

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

#define NUM_STUDENTS 3
#define MAX_PROBLEM_1_SCORE 30
#define MAX_PROBLEM_2_SCORE 30
#define MAX_PROBLEM_3_SCORE 40

#define MAX_SCORE (MAX_PROBLEM_1_SCORE + MAX_PROBLEM_2_SCORE + MAX_PROBLEM_3_SCORE)

typedef struct student {
    size_t id;
    unsigned int prob_1;
    unsigned int prob_2;
    unsigned int prob_3;
    unsigned int total_score;
    char grade;
} student;

void cal_total_score(student* s, size_t length);
void student_bb_sort(student* s, size_t length);

int main() {
    student s[NUM_STUDENTS];
    unsigned int total_score;
    size_t i;

    const float lower_grade_threshold = ((float) MAX_SCORE) / 3;
    const float upper_grade_threshold = ((float) MAX_SCORE) * 2.0 / 3.0;

    srand(time(NULL));

    for (i = 0; i < NUM_STUDENTS; i++) {
        s[i].id = i;
        s[i].prob_1 = rand() % MAX_PROBLEM_1_SCORE + 1;
        s[i].prob_2 = rand() % MAX_PROBLEM_2_SCORE + 1;
        s[i].prob_3 = rand() % MAX_PROBLEM_3_SCORE + 1; 

        cal_total_score(&s[i], NUM_STUDENTS);
        
        printf("Student ID: %d\tP1: %d\tP2: %d\tP3: %d\tScore: %d\n",
               s[i].id, 
               s[i].prob_1, 
               s[i].prob_2, 
               s[i].prob_3, 
               s[i].total_score);
    }

    student_bb_sort(s, NUM_STUDENTS);
    printf("\nAfter Bubble Sort\n");

    for (i = 0; i < NUM_STUDENTS; i++) {
        if (i < NUM_STUDENTS / 3) {
            s[i].grade = 'A';
        } else if (i < 2 * NUM_STUDENTS / 3) {
            s[i].grade = 'B';
        } else {
            s[i].grade = 'C';
        }

        printf("Student ID: %d\tScore: %d\tGrade: %c\n", 
                s[i].id, 
                s[i].total_score, 
                s[i].grade);
    }
}

void cal_total_score(student* s, size_t length) {
    int i;
    for (i = 0; i < length; i++) {
        s[i].total_score = s[i].prob_1 + s[i].prob_2 + s[i].prob_3;
    }
}

void student_bb_sort(student* s, size_t length) {
    student tmp_student;
    int i, j;
    int swapped;

    do {
        swapped = 0;

        for (i = 1; i < length; ++i) {
            if (s[i - 1].total_score < s[i].total_score) {
                swapped = 1;
                tmp_student = s[i];
                s[i] = s[i - 1];
                s[i - 1] = tmp_student;
            }
        }
    } while (swapped);
}

Answer 3

Here is what I think: Do not use bubble sort(BS), except it is your teacher's requirement. Because the time complexity for BS is n*n (n are the elements) if you want to sort

typedef struct student {
    int id;
    int prob_1;
    int prob_2;
    int prob_3;
    int total_score;
    char grade;
}Student;

// add comparator
bool operator<(const Student&  st1, const Student&  st2){
    return st1.total_score < st2.total_score; // please check whether it is  < or >
}

now if you have a vector<Student> vec; you can quickly sort it by built-in sort
The next thing is I do not know the benefit of using size_t except to save a little memory (If someone knows anything else please tell me).
But if you want to use size_t or unsigned int, you really should be careful with the problem: integer overflow, especially where the input from the users. For example:
Matrix(x,y); // when x and y are the input from users, and their data types are size_t or unsigned.
and if you expect that when the user's input is Matrix(-1, 2), it gives you an error or something so you are wrong, the program runs, and -1 will be the positive max number of size_t.

The last thing is: to try not to pass a pointer, instead use pass by reference.

Answer 4

Including some specification in cal_total_score() & student_bb_sort() is the way to go, credit going to the creators of the "problem" or a solution.

read [the source code of main()] if you need it

I need it to interpret In each student, total_score must be the sum of prob_1, prob_2, and prob_3.:
This function is called NUMSTUDENT times, with the address of each student in s in succession - looks like a better wording of the requirement was

// Problem (A): Implement the function "cal_total_score".
/** In Student s, set total_score to the sum of prob_1, prob_2, and prob_3. */
void cal_total_score(Student* s) {
    s->total_score = s->prob_1 + s->prob_2 + s->prob_3;
}

(The original requirement could as easily be fulfilled with
s->prob_3 = s->total_score - s->prob_1 - s->prob_2; -
s->total_score = s->prob_1 = s->prob_2 = s->prob_3 = 0; doesn't even need calculation. )

Iterating NUMSTUDENT times in cal_total_score() does all the work useful in the first call.
Starting with i= 1, the call cal_total_score(&s[i]); (which I'd still code cal_total_score(s + i);)
results in access to s[NUMSTUDENT] - beyond the array, "undefined behaviour": anything may happen.

---

Implementing bubble sort is reinventing the wheel - won't comment without the point of the exercise stated.
Most of what I think weird is in the code that seems to have been given (typedef struct student Student;?).