2012-10-06 32 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; 
    } 
} 
相关问题