4
\$\begingroup\$

I have a file containing information about the session of students. The structure of this file is: group number, student name, exams, and tests.

  • debt() function find and print the names of underachieving students.
  • student_score() calculate the average score of groups
  • group_score() calculate the average score of each student.

So, this is my program and maybe I can make something better?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <wchar.h>
#include <locale.h>

#define N 50

void debt();
void student_score();
void group_score();

struct STUDENTS {
  int group;
  char name[100];
  char test[15];
  int exam;
};

struct STUDENTS student_list[N];

int main() {
  FILE *file;
  int choice;
  char buffer[200];
  int i = 0;

  file = fopen("students.txt", "r");

  if (file == NULL) {
    puts("students.txt не найден");
    return 1;
  }

  while (fgets(buffer, sizeof buffer, file) != NULL) {
    sscanf(strdup(buffer), "%d %[^0-9] %d %s", &student_list[i].group, student_list[i].name, &student_list[i].exam, student_list[i].test);
    i++;
  }
  fclose(file);
  int student_list_size = i;

  do {
    puts("\nВыберите номер пункта меню:");
    puts("1 - Cтуденты, имеющие задолженности");
    puts("2 – Средний балл каждого студента");
    puts("3 – Средний балл всей группы");
    puts("4 - Выход\n");

    scanf("%d", &choice);

    switch (choice) {
      case 1:
        debt(student_list_size);
        break;
      case 2:
        student_score(student_list_size);
        break;
      case 3:
        group_score(student_list_size);
        break;
      case 4:
        break;
      default:
        puts("\nНужно вводить номер пункта от 1 до 4");
        break;
    }
  } while (choice != 4);

  return 0;
}

void debt(int student_list_size) {
  int counter_debt;
  wchar_t test_wchar[15];
  wchar_t * pch;

  setlocale(LC_ALL, "");

  for (int i = 0; i < student_list_size; i++) {
    counter_debt = 0;
    mbstowcs(test_wchar, student_list[i].test, 15);

    pch = wcschr(test_wchar, L'н');
    while (pch != NULL) {
      counter_debt++;
      pch = wcschr(pch + 1, L'н');
    }

    if (counter_debt > 0) {
      printf("%d %s: %d\n", student_list[i].group, student_list[i].name, counter_debt);
    }
  }
}

void student_score(int student_list_size) {
  int exam;
  int average;
  int digit;
  int counter_digits;

  for (int i = 0; i < student_list_size; i++) {
    average = 0;
    digit = 0;
    counter_digits = 0;
    exam = student_list[i].exam;

    while (exam) {
      digit = exam % 10;
      average += digit;
      counter_digits++;
      exam = exam / 10;
    }

    printf("%d %s: %.1f\n", student_list[i].group, student_list[i].name, (double)average / counter_digits);
  }
}

void group_score(int student_list_size) {
  int exam;

  struct GROUP_SCORE {
    int group_score;
    int students_counter;
  };
  struct GROUP_SCORE group_list[5000] = {0};

  for (int i = 0; i < student_list_size; i++) {
    group_list[ student_list[i].group ].students_counter += 1;
    exam = student_list[i].exam;
    while (exam) {
      group_list[ student_list[i].group ].group_score += exam % 10;
      exam = exam / 10;
    }
  }

  for (int j = 0; j < 5000; j++) {
    if (group_list[j].group_score > 0) {
      printf("%i: %.1f\n", j, (double)group_list[j].group_score / group_list[j].students_counter);
    }
  }
}

students.txt:

4272 Галкин Г. А. 5445 ззззз
4273 Константинопольский А. А. 4333 знзнз
4273 Курочкин А. А. 3433 знзнн
4272 Козлов И. И. 4443 ззззн
\$\endgroup\$
  • \$\begingroup\$ What's specifically your question? \$\endgroup\$ – Sam Delaney Dec 28 '14 at 12:44
  • \$\begingroup\$ @SamDelaney what I can make better \$\endgroup\$ – rel1x Dec 28 '14 at 12:57
  • \$\begingroup\$ the first thing to do would be fix the compiler raised errors and warnings. \$\endgroup\$ – user3629249 Dec 28 '14 at 13:35
  • \$\begingroup\$ @user3629249 I found no compiler errors with a C99 compiler. What was one error you found? \$\endgroup\$ – chux Dec 28 '14 at 14:46
  • \$\begingroup\$ @user3629249 I haven't error messages. I use Mac OS X 10.10.1 and GCC Apple LLVM version 6.0 (clang-600.0.56) \$\endgroup\$ – rel1x Dec 28 '14 at 15:15
3
\$\begingroup\$
  1. Completely prototype functions: debt() student_score() group_score();

    // void debt();
    void debt(int student_list_size);
    
  2. Avoid naked magic numbers. Use #define or a constant to self document various numbers.

    #define TEST_N (15)
    struct STUDENTS {
      ...
      char test[TEST_N];
    };
    
    void debt(int student_list_size) {
      // wchar_t test_wchar[15];
      wchar_t test_wchar[TEST_N];
      ...
      // mbstowcs(test_wchar, student_list[i].test, 15);
      mbstowcs(test_wchar, student_list[i].test, TEST_N);
    
  3. Use size_t for array index types and the result of sizeof. int may not be the right size to use as an index. (Also i in main())

     // void student_score(int student_list_size) {
     void student_score(size_t student_list_size) {
       ...        
       // for (int i = 0; i < student_list_size; i++) {
       for (size_t i = 0; i < student_list_size; i++) {
    
  4. Localize variables. This approaches a style issue, but declaring variables close to their use has value.

    void student_score(int student_list_size) {
      // int exam;
      // int average;
      // int digit;
      // int counter_digits;
    
      for (int i = 0; i < student_list_size; i++) {
        int exam;
        int average;
        int digit;
        int counter_digits;
    
  5. The strdup() serves no purpose. Eliminate it.

    // sscanf(strdup(buffer), "%d ...
    sscanf(buffer, "%d ...
    
  6. The return value of sscanf() should be checked and string widths limited. Note: student_list[i].name will have a ' ' at the end.

    // sscanf(buffer, "%d %[^0-9] %d %s", &student_list[i].group, ...
    if (4 != sscanf(buffer, "%d %99[^0-9] %d %14s", &student_list[i].group, ...) {
       Handle_Format_Error();
    
  7. Use valid main() signature.

    int main() {
    int main(void) {
    
  8. Conversions char to/from wchar_t may need different sizes.

  9. The extra '\n' in puts("4 - Выход\n"); looks wrong. Guess I am not a fan of puts(). Why not use 1 function call?

    fputs("\nВыберите номер пункта меню:\n"
          "1 - Cтуденты, имеющие задолженности\n"
          "2 – Средний балл каждого студента\n"
          "3 – Средний балл всей группы\n"
          "4 - Выход\n"
          "\n", stdout);
    
\$\endgroup\$
  • \$\begingroup\$ this 'int main(void) {' is NOT a valid main declaration \$\endgroup\$ – user3629249 Dec 28 '14 at 14:48
  • \$\begingroup\$ placing the variables inside the 'for' loop will result in the manipulation of the stack to push/pop those variables in each iteration of the 'for' loop This results in unnecessary code and takes CPU cycles. \$\endgroup\$ – user3629249 Dec 28 '14 at 14:51
  • \$\begingroup\$ @user3629249 int main(void) is valid. "... It shall be defined with a return type of int and with no parameters: int main(void) ... or with two parameters ..." C11 5.1.2.2.1 1 \$\endgroup\$ – chux Dec 28 '14 at 17:37
  • \$\begingroup\$ @user3629249 1) C certainly does not specify the push/pop activity in your comment. 2) A good compiler would not do so anyways. 3) The dominate activity is the while loop inside. That is where any optimizations should be had. \$\endgroup\$ – chux Dec 28 '14 at 17:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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