Opened 3 years ago

Closed 3 years ago

#76 closed ожидается проверка (задача сдана)

WW_3_kravchenko

Reported by: kravchenko.egor Owned by: Святослав Власов
Component: WW_mergesort Version: 3.0
Keywords: Cc:

Description


Change History (7)

comment:1 Changed 3 years ago by Святослав Власов

Корректность - 0/7.
Вывод не должен содержать лишних пробельных символов (у тебя содержит в конце), из-за этого не могу проверить корректность.

Стиль - 0/3
Замечания:

  1. Лучше выделить слияние отсортированных массивов в отдельную функцию merge.
  2. Определись, ставишь ли ты пробел между ключевым словом и скобкой или нет в конструкциях if и for и приведи код к одному стилю. Пробел после открывающей скобки ставить в любом случае не нужно.
  3. В конструкциях вроде такой
if (mergesort(array, num1, element_size, comparator) == -1 || 
  mergesort(array + num1 * element_size, num2, element_size, *comparator) == -1)
  return -1;

лучше вторую строку в условии сдвинуть на два отступа, чтобы визуально отделить её от блока с кодом. Это раз. А во-вторых если условие или блок кода занимают больше одной строки, то блок кода следует заключать в фигурные скобки. Этого требуют все приличные кодстайлы. Должно получиться вот так:

if (mergesort(array, num1, element_size, comparator) == -1 || 
    mergesort(array + num1 * element_size, num2, element_size, *comparator) == -1) {
  return -1;
}
  1. Длинные сигнатуры функций (длиннее 80 символов) лучше разбивать на несколько строк для лучшей читаемости. Например, сигнатуру твоей функции mergesort лучше записать вот так:
int mergesort(void *array, size_t elements, 
              size_t element_size, 
              int (*comparator)(const void*, const void*)){
  ...
}
  1. Между закрывающей фигурной скобкой и else не нужно ставить пробелов. Да и переносить строку тоже не стоит. Оставлять пустую строку между условием и блоком кода не нужно, то же самое касается пустых строк перед закрывающейся фигурной скобкой. Наиболее используемый вариант записи структуры if-else вот такой:
    if (...) {
      <code>
    } else if (...) {
      <code>
    } else {
      <code>
    }
    

То же самое касается while, for и других подобных конструкций.

  1. Указатель на функцию не нужно разыменовывать для вызова, вместо (*comparator)(first, second) можно просто писать comparator(first, second).
  2. Не обязательно заводить отдельные переменные для того, чтобы вызвать от них функцию. Иногда это можно делать для повышения читаемости, но вот такая конструкция явно избыточна:
    char *to = array;
    char *from = res;
    copy(to, from, element_size * elements);  
    

В остальных случаях я бы тоже предпочел не заводить отдельных переменных. Если ты хочешь для повышения читаемости обозначить имена аргументов, то это можно сделать с помощью комментариев. Вот пример хорошо проаннтированного вызова функции:

copy(/* to */ res + cur * element_size,
     /* from */ array + (j + num1) * element_size,
     element_size);
  1. Не нужно ставить много пустых строк подряд. Больше двух точно никогда не нужно. Это размазывает код и затрудняет его чтение.
  2. Не нужно писать свою cmp_str, можно воспользоваться библиотечной strcmp.
  3. Вот такой вариант cmp_int смотрится элегантнее:
    return *(int*)a - *(int*)b;
    
  4. Вместо того, чтобы пихать весь код под условие if (argc >= 3), лучше написать
    if (argc < 3)
      return 0;
    <остальной код>
    
  5. Вместо того, чтобы сравнивать первую букву в имени типа данных, лучше всё же по-честному написать strcmp(type, "int")
  6. Не нужно пользоваться argv как указателем, лучше использовать его как массив: argv[i]
  7. То же самое касается остальных массивов в main.c. У тебя на этом этапе известны типы данных, поэтому нет нужды использовать сложную неочевидную арифметику указателей, лучше пользоваться синтаксисом массива, например в заполнении массива интов разумно написать
    for (int i = 2; i < argc; ++i) {
      arr[i - 2] = atoi(argv[i]);
    }
    
Last edited 3 years ago by Святослав Власов (previous) (diff)

comment:2 Changed 3 years ago by Святослав Власов

Type: ожидается проверкаожидаются исправления

comment:3 Changed 3 years ago by Святослав Власов

Насчет лишних пробелов -- исправили проблему, теперь тесты работают.
Тесты проходят, к корректности претензий нет, но за лишние пробелы всё таки минус балл, поэтому 6/7

Last edited 3 years ago by Святослав Власов (previous) (diff)

comment:4 Changed 3 years ago by kravchenko.egor

Type: ожидаются исправленияожидается проверка
Version: 1.02.0

comment:5 Changed 3 years ago by Святослав Власов

Type: ожидается проверкаожидаются исправления

Корректность 6/7:

  1. Сортировка строк поломалась
  2. Вывод обязательно должен завершаться пустой строкой

Стиль 2/3:

  1. Не надо ставить пустые строки в начале тела функции.
  2. Вот так лучше не писать: if (res == NULL) return -1; тело лучше перенести на новую строку
  3. Пробел перед ++ не ставится.
  4. А вот между ) и { пробел обычно ставится.

comment:6 Changed 3 years ago by kravchenko.egor

Type: ожидаются исправленияожидается проверка
Version: 2.03.0
  1. Сортировку строк пофиксил -- очень глупый баг был.
  2. Не очень понял, что нужно сделать. Надеюсь, просто вывести после всего вывода пустую строку. В задании не было об этом ничего написано. Это общее требование ко всем консольным приложениям?

Есть какая-нибудь программа, проверяющая стиль? На странице курса про него совсем мало написано, а поиск и исправление этих ошибок (особенно тех, про которые я раньше не знал) занимают время и у меня и у Вас.

comment:7 Changed 3 years ago by Святослав Власов

Resolution: задача сдана
Status: assignedclosed
  1. Сортировку строк не пофиксил. Посмотри внимательно в задании -- тип называется str, а не string.
  2. Согласен, в задании не объяснено, поэтому за это баллы не снижаю, на практике я к этому вернусь и всё уточню.

Т.к. строки не сортируются, но ошибка элементарно фиксится, поэтому за корректность 6.5/7

Стиль 2.8/3
Код выглядит намного лучше, чем изначально. Из недочетов -- кое где остались переводы строк перед else, затесался уродливый for в main.c, кое где остались конструкции if (...) return ...;

Еще критичная ошибка -- в конце функции mergesort ты не возвращаешь значение, а значит в этом случае вернется какой-то мусор, который по счастливой случайности может оказаться равен -1 и программа отработает неправильно.
Но я проглядел эту ошибку в предыдущих итерациях, поэтому баллы не снимаю.

Итого 9.3/10, плюс доп. балл за бонус, итого 10.3/11

Насчет программ проверяющих стиль: есть классические форматтеры clang-format и astyle.
Некоторые IDE позволяют настроить автоматическое форматирование кода при сохранении файла или по запросу, например https://marketplace.visualstudio.com/items?itemName=xaver.clang-format
Можно этим пользоваться. Но лучше самому привыкать писать чистый и красивый код.

Note: See TracTickets for help on using tickets.