2012-10-06 28 views
0

我有一個結構,爲什麼這會一直返回我單向鏈表中的一個節點?

typedef struct song{ 
    string title; 
    string artist; 
    string album; 
    string genre; 
    float rating; 
    struct song *next; 
    }song_t; 

和刪除功能,

song_t *DeleteSong(song_t *head, string key){ 
    song_t *temp, *temp2, *holder; 
    holder = (song_t *)malloc(sizeof(song_t)); 
    temp = head; 


    while(temp != NULL && strcasecmp(temp->title, key) != 0){ 
     temp = temp->next; 
    } 
    if(temp == NULL){ 
     newline; 
     printf("Song not found."); 
     newline; 
     newline; 
    } 

    else if(temp == head){ 
     if(temp->next == NULL){ 
      temp->next = NULL; 
      free(temp); 
      return NULL; 
     } 
     else{ 
     head = temp->next; 
     } 
    } 

    else if(temp->next == NULL){ 
     temp2 = head; 

     while(temp2->next->next != NULL){ 
      temp2 = temp2->next; 
     } 
     holder = temp2->next->next; 
     temp2->next = NULL; 
    } 

    else{ 
     temp2 = head; 
     while(temp2->next != temp){ 
      temp2 = temp2->next; 
     } 
     holder = temp; 
     temp2->next = temp->next; 
     free(holder); 
    } 
    return head; 
    } 

,最後一個刪除重複的功能,

song_t *RemoveDuplicate(song_t *head){ 
    song_t *temp; 
    song_t *temp2; 
    temp = head; 
    temp2 = temp->next; 

    while(temp != NULL){ 
     while(temp2 != NULL){ 
      if(strcasecmp(temp->title,temp2->title) == 0 && strcasecmp(temp->artist,temp2->artist) == 0 && strcasecmp(temp->album,temp2->album) == 0 && strcasecmp(temp->genre,temp2->genre) == 0){ 
       if(temp2 == temp->next){ 
        temp = DeleteSong(temp,temp2->title); 
       } 
       else{ 
        temp2 = DeleteSong(temp2->next,temp2->title); 
       } 
      } 
      temp2 = temp2->next; 
     } 
     temp = temp->next; 
    } 

    } 

但是,每當我包括刪除重複的功能主要, head = RemoveDuplicate(head);

結果總是隻返回一個結構並刪除整個列表。我認爲RemoveDuplicate函數有問題,因爲我測試了DeleteSong函數,並且它工作正常。

+0

在else聲明我不明白你爲什麼要傳遞temp2-> next作爲頭指針。 並且在'temp = temp-> next'(內部while之後)不應該放置'temp2 = temp-> next;'? –

回答

0

如果在列表中的前兩個條目是重複的,那麼就好像你第一次檢查:

if(temp2 == temp->next){ 
    temp = DeleteSong(temp,temp2->title); 
} 

它將在列表的頭部計算爲真。如果您的列表突然減少到列表的頭部,您可能想要檢查從那裏發生的情況。

我懷疑這個錯誤實際上可能在DeleteSong函數中找到,因爲它比我從其他我看過的單鏈表中看起來有點複雜。

在僞C,我可能會嘗試這樣的事:

to_delete = find_node_by_key(key) 
return if to_delete == null 

current = head 
last = null 

if to_delete == current 
{ 
    head = current->next 
    to_mem_free = current 
} 
else do 
{ 
    if to_delete == current 
    { 
     to_mem_free = current 
     if (last != null) last->next = current->next 
     break 
    } 

    last = current 

} while (current = current->next) != null 

您可以刪除與實際刪除節點把搜索,所以你不會有遍歷列表兩次。您也可以嘗試避免使用「temp」作爲變量名稱,除非絕對必要,因爲它經常會導致混淆。

許多工具鏈包括提供單鏈表的庫。您可以通過使用這樣一個庫來節省編碼和調試時間,這可能還會提供排序樹或散列表,以顯着加快對歌曲標題的搜索速度。

0
holder = (song_t *)malloc(sizeof(song_t)); 
/* snip */ 
/* else if */ 
    holder = temp2->next->next; 
/* else */ 
    holder = temp; 
    temp2->next = temp->next; 
    free(holder); 

當分配holder = temp2->next->next;(順便說一句,這總是NULL那裏),或holder = temp;,你失去你的參考malloc版內存。由於您根本沒有真正使用holder,因此修復方法是將其從功能中刪除。在第一種情況下,除非指定複雜的NULL,否則在第二種情況下除去holder = temp;free ing temp是正確的方法。

有一些更奇怪的事情和錯誤:

song_t *DeleteSong(song_t *head, string key){ 
    song_t *temp, *temp2, *holder; 
    holder = (song_t *)malloc(sizeof(song_t)); // as said, remove holder completely 
    temp = head; 

    /* Find song with given title */ 
    while(temp != NULL && strcasecmp(temp->title, key) != 0){ 
     temp = temp->next; 
    } 
    if(temp == NULL){ 
     newline; 
     printf("Song not found."); 
     newline; 
     newline; 
    } 
    /* It's the very first in the list */ 
    else if(temp == head){ 
     if(temp->next == NULL){ 
      /* It's even the only one */ 
      temp->next = NULL; // This runs only if temp->next is already NULL 
      free(temp);   // Also free the members of temp, or you're leaking 
      return NULL; 
     } 
     else{ 
      head = temp->next; // You should now free temp and members, or you're leaking memory 
     } 
    } 
    /* It's the last one in the list, but not the first */ 
    else if(temp->next == NULL){ 
     temp2 = head; 
     /* Find the penultimate song */ 
     while(temp2->next->next != NULL){ 
      temp2 = temp2->next; 
     } 
     holder = temp2->next->next; 
     temp2->next = NULL; // You should now free temp and members, or you're leaking memory 
    } 
    /* Neither first nor last */ 
    else{ 
     temp2 = head; 
     /* Find song before */ 
     while(temp2->next != temp){ 
      temp2 = temp2->next; 
     } 
     holder = temp; 
     temp2->next = temp->next; 
     free(holder); 
    } 
    return head; 
} 

但除了泄漏,它是正確的,但不必要的複雜。

song_t *DeleteSong(song_t *head, string key) { 
    song_t *prev = NULL, curr = head; 
    /* Find song and node before that */ 
    while(curr != NULL && strcasecmp(curr->title, key) != 0) { 
     prev = curr; 
     curr = curr->next; 
    } 
    if (curr == NULL) { 
     /* Not found */ 
     newline; 
     printf("Song not found."); 
     newline; 
     newline; 
    } else if (prev == NULL) { 
     /* It's the very first song in the list 
     * so let head point to its successor 
     * and free the song; it doesn't matter 
     * if it's the last in the list 
     */ 
     head = curr->next; 
     free(curr->title); // Probably, but not if title isn't malloced 
     free(curr->artist); // Ditto 
     free(curr->album); 
     free(curr->genre); 
     free(curr); 
    } else { 
     /* We have a predecessor, let that point 
     * to the successor and free the song 
     */ 
     prev->next = curr->next; 
     free(curr->title); // See above 
     free(curr->artist); 
     free(curr->album); 
     free(curr->genre); 
     free(curr); 
    } 
    return head; 
} 

另一方面,您的RemoveDuplicate函數不會做你想要的。除了複製件緊跟在原件之後的情況和沒有複製件的情況之間的區別之外,我可以看到沒有任何理由,作業temp = DeleteSong(temp,temp2->title);或相應的作業。 temp2 = DeleteSong(temp2->next,temp2->title);改變什麼temp resp。 temp2指向,但不是列表中相應的前任指向的地方。讓我們舉例說明一個小ASCII藝術的問題:

 temp temp2 
     |  | 
     v  v 
song1->song2->song3->song4->song5->... 

其中song2song3是重複的。現在temp = DeleteSong(temp,temp2->title);,因爲兩首歌是重複的DeleteSonghead說法已經匹配,並在你的DeleteSong版本,head = temp->next; return head;是所有做的,所以名單都沒變,你只會得到

  temp temp2 
       | | 
       v v 
song1->song2->song3->song4->... 

兩個指向同一個列表節點的指針。在釋放節點的DeleteSong版本中,song1.next現在將指向freed內存,ouch。

如果重複不立即按照原來的,temp2 = DeleteSong(temp2->next,temp2->title);可能無法找到匹配的標題歌曲,因爲搜索已知的匹配,在這種情況下,列表不會被修改,在所有後開始temp2是隻是改變爲指向繼任者。如果在此之後有歌曲匹配標題,則發現的副本仍未從列表中刪除,並且之後的部分可能會更改,可能會導致狀態不正確。如果temp2指向列表中的倒數第二個節點,並且最後一個節點具有匹配的標題,則最後一個節點爲freed,但next指針的重複內容未更改,因此現在指向freed內存,你有一個懸掛指針(這是一個段等待發生)。

的另一個問題是,在DeleteSongRemoveDuplicate去除的標準是不同的,前者,你只檢查稱號,後者也藝術家,專輯和流派,所以在使用前函數在後者的風險刪除不應該刪除的歌曲(考慮封面版本)。

當你想從一個單獨的鏈表中刪除一個節點時,你需要一個指向前一個節點的指針來改變那些指向的內容,否則你將創建懸掛指針。我已經給了一個標準的方法,那上面做,你基本上可以複製到內循環,但在這裏它可以永遠不會發生,我們要刪除的第一個節點,所以這是一個有點簡單:

// void, since head is never changed, only duplicates after the original are removed 
void RemoveDuplicates(song_t *head) { 
    song_t *orig = head, prev, curr; 
    while(orig != NULL && orig->next != NULL) { 
     prev = orig; 
     curr = orig->next; 
     while(curr != NULL) { 
      // Find next song to remove 
      while(curr != NULL && !meets_deletion_criteria(orig, curr)) { 
       prev = curr; 
       curr = curr->next; 
      } 
      // Now either curr is NULL, or it shall be deleted 
      if (curr != NULL) { 
       // Let the predecessor point to curr's successor 
       prev->next = curr->next; 
       clean_up(curr); // free all malloced members and the node itself 
       curr = prev->next; 
      } 
     } 
     orig = orig->next; 
    } 
} 
相關問題