Change History (5)

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

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

Корректность -- 7/7

Стиль -- 0/3

  1. Длинные строки (>80 символов) нужно разбивать на несколько. Примеры того, как строки должны разбиваться для условий, вызовов функций, объявлений функций и т.д. можешь посмотреть вот тут -- https://google.github.io/styleguide/cppguide.html#Formatting
  2. num_elem можно написать намного проще и даже обойтись без отдельной функции. Тебе совсем не нужен цикл. Ты знаешь размер куска памяти, знаешь размер одного элемента, найти количество элементов -- задача из 2-го класса школы.
  3. То же самое касается функции elem_pointer -- там не нужен цикл, можно обойтись адресной арифметикой.
  4. Зачем тебе две переменные result и sv_res?
  5. Не нужно писать одни функции внутри других -- пиши их все в глобальной области видимости.
  6. Зачем ты в merge передаешь array, если его никак не используешь?
  7. Тут тоже можно использовать адресную арифметику:
    while (j < elem_size){
      j++;
      ptr++;
    }
    
  8. Блок else тут избыточен:
    if (ch1 == 0 && ch2 == 0 && ch3 == 0){
      return 0;
    } else{
      return -1;
    }
    

вместо этого достаточно написать

if (ch1 == 0 && ch2 == 0 && ch3 == 0){
  return 0;
}
return -1;
  1. Зачем ты выделяешь память в right_bound и затем никак её не используешь?
  2. stdlib.h в mergesort.h избыточна. Тебе оттуда нужен только size_t, он объявлен в stddef.h. А stdlib.h можешь подключить в mergesort.c

В целом код получился очень избыточным. Постарайся сделать его более опрятным и лаконичным. Убери ненужные переменные, ненужные аргументы функций, ненужные функции.
К примеру, зачем тебе нужна функция mergeSortRecursive? Единственное её отличие от исходной функции mergesort только в том, что принимает в качестве аргументов указатель на начало и конец массива вместо указателя на начало и массива и количество аргументов. Но ведь эти вещи легко конвертируются друг в друга. Можно вместо mergeSortRecursive пользоваться самой mergesort.

Касательно main.c:

  1. Не нужно выделять память под входные данные на стеке. Стек ограничен по размеру, если входных данных будет очень много, стек переполнится.
  2. Вместо того, чтобы весь код main пихать под условие if (n > 0) лучше написать:
    if (n <= 0)
      return 1;
    <остальной код>
    
  3. Нет смысла копировать указатели на строки в отдельный массив, можно просто передать в функцию argv
  4. В конце вывода забыла про перевод строки.
Last edited 3 years ago by Святослав Власов (previous) (diff)

comment:2 Changed 3 years ago by predelina.anastasiya

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

Внесла поправки по замечаниям.

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

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

Стиль -- 2/3

  1. Не проще ли будет в swap написать цикл-for, вместо цикла-while? Да и вообще, нужно ли тебе именно менять два элемента местами?
  2. free(NULL) -- бессмысленная операция.
  3. Если в первом вызове mergesort память не выделится, то нет никакого смысла запускать вторую итерацию и merge, надо сразу выходить.
  4. ch1, ch2, ... -- это не самые лучшие имена для возвращаемых значений, лучше уж ret или err.
  5. Не нужно жалеть пустых строк, стоит расставлять их в коде для обозначения логических пауз и выделения логических блоков. У тебя их маловато.
  6. Функции swap и merge в заголовочном файле не нужны. Это внутренние детали реализации и во внешних файлах вызываться не будут.

В целом код стал выглядеть намного-намного лучше, молодец!

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

comment:4 Changed 3 years ago by predelina.anastasiya

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

Поправила

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

Resolution: задача сдана
Status: assignedclosed

11/11, молодец!

Note: See TracTickets for help on using tickets.