Мой учитель сказал мне, что мой код был мусором, почему?

Question

Есть большая разница между этими двумя алгоритмами?

Это мой:

void MergeList(sqList la,sqList lb,sqList *lc){
  int j,k,n=0;

  for (j = la.length - 1, k = lb.length - 1; j >= 0 && k >= 0; ){
    if (*(la.elem + j) > *(lb.elem + k)){
      *(lc->elem + n) = *(la.elem + j);
      j--;
    } else if(*(la.elem + j) < *(lb.elem + k)){
      *(lc->elem + n) = *(la.elem + k);
      k--;
    } else {
      *(lc->elem + n) = *(la.elem + j);
      j--;
      k--;
    }
    n++;
    lc->length++;
  } 

  for ( ;j >= 0; j--){
    *(lc->elem + n) = *(la.elem + j);
    lc->length++;
  }

  for ( ;k >= 0; k--){
    *(lc->elem + n) = *(lb.elem + k);
    lc->length++;
  }
}

Это из книги.

void MergeList(sqList la,sqList lb,sqList *lc){
  ElemType *pa,*pb,*pc,*pa_first,*pb_first;

  pa = la.elem + la.length - 1;
  pb = lb.elem + lb.length - 1;
  pc = lc->elem;

  pa_first = la.elem;
  pb_first = lb.elem;

  while(pa >= pa_first && pb >= pb_first){
    if(*pa > *pb)
      *pc++ = *pa--;
    else if(*pa < *pb)
      *pc++ = *pb--;
    else{
      *pc++ = *pa-- = *pb--;

    }
    lc->length++;

  }

  while(pa >= pa_first)
  *pc++ = *pa--;
  while(pb >= pb_first)
  *pc++ = *pb--;
}

Answer 1

Мой рефакторинг ...

Изменения сделаны:

• сделано MergeList вернуть список, чтобы вызывающий мог сделать такие вещи, как resultList = MergeList(..., чтобы сделать егогораздо более очевидно, что произошло.

• изменили имена переменных, чтобы они были более наглядными

• добавили "пробел перед левой скобкой" везде

• везде добавлено "пробел после запятой"

• везде добавлено "пробел вокруг операторов"

• замененовычисления указателя с поиском в массиве (см. комментарий Джереми Фризнера)

• заменили чрезмерно сложные for() shenanigans (использование запятой, пустые части) на более простые / легкие для чтения циклы

• добавлена ​​документация (до запуска функции)

• добавлены комментарии в функции

• добавлено const в списки источников

Код:

// Construct a new list sorted by descending order of "elem" by merging data from
// two existing lists that are already sorted by descending order of "elem".
//
// WARNINGS:
//   If both source lists are not sorted the resulting list will not be sorted.
//   Caller MUST ensure that enough memory is allocate for the resulting list before calling this function.

sqList *MergeList(const sqList list_a, const sqList list_b, sqList *result_list) {
  int j, k, n = 0;

  // Do entries from both source lists until there's nothing left in at least one of the source lists

  j = list_a.length - 1;
  k = list_b.length - 1;
  while(j >= 0 && k >= 0) {
    if (list_a.elem[j] > list_b.elem[k]) {
      result_list->elem[n] = list_a.elem[j];
      j--;
    } else if(list_a.elem[j] < list_b.elem[k]) {
      result_list->elem[n] = list_a.elem[k];
      k--;
    } else {
      result_list->elem[n] = list_a.elem[j];
      j--;
      k--;
    }
    n++;
    result_list->length++;
  } 

  // If all entries in list_a haven't been consumed, copy the remaining entries to the result list

  while(j >= 0) {
    result_list->elem[n] = list_a.elem[j];
    result_list->length++;
    j--;
  }

  // If all entries in list_b haven't been consumped, copy the remaining entries to the result list

  while (k >= 0) {
    result_list->elem[n] = list_b.elem[k];
    result_list->length++;
    k--;
  }

  return result_list;
}

Answer 2

Есть большая разница между этими двумя алгоритмами?

Они не делают то же самое, поэтому да, есть большая разница

В вашем коде есть ошибка, которую я считаю классической ошибкой копирования и вставки.Эта строка неверна:

} else if(*(la.elem + j) < *(lb.elem + k)){
  *(lc->elem + n) = *(la.elem + k);    // ERROR... la.elem should be lb.elem
  k--;
} else {

Кроме того, расчет lc->length отличается.В этом коде из книги:

while(pa >= pa_first)
    *pc++ = *pa--;

нет приращения, но в вашем коде

  for ( ;j >= 0; j--){
    *(lc->elem + n) = *(la.elem + j);
    lc->length++;
  }

есть.Итак, снова есть разница.Я предполагаю, что книга не права, а вы правы.

Код в книге уменьшает указатель и сравнивает его с указателем на первый элемент.Поскольку уменьшенный указатель в конечном итоге будет указывать перед первым элементом, код имеет неопределенное поведение.

Мой учитель сказал мне, что мой код был нежелательным. Почему?

Aучитель не должен этого говорить .... и учитель должен хотя бы объяснить, почему.

При оценке кода можно сосредоточиться на нескольких вещах:

• Функциональностьто есть выполняет ли он то, что должен - это высший приоритет

• Производительность, т.е. выполняет ли код с необходимой вам скоростью

• Удобство обслуживания, т. Е. Код легко читается, понимается и поддерживается.

Ваш код нелегко прочитать, но, поскольку код в книге неправильный, я будувсе еще предпочитаете ваш код (после исправления ошибки опечатки).

Один простой пример, который вы можете изменить, чтобы повысить читабельность:

*(la.elem + j) --> la.elem[j]

Также обратите внимание, что в книге есть код, который не простчитать.Посмотрите на эту строку из книги:

*pc++ = *pa-- = *pb--;

Как читатель кода, можно только подумать: «что этот код должен делать?»