Why does my C++ LinkedList method print out the last word more than once?

Question

When I call the cmremoveNode method in my LinkedList from outside code, I get an EXC_BAD_ACCESS.

FIXED: But now the last word using the following test code gets repeated twice:

#include <iostream>
#include "LinkedList.h"
using namespace std;

int main (int argc, char * const argv[]) {
    ctlinkList linkMe;
 linkMe.cminsertNode("The");
 linkMe.cminsertNode("Cat");
 linkMe.cminsertNode("Dog");
 linkMe.cminsertNode("Cow");
 linkMe.cminsertNode("Ran");
 linkMe.cminsertNode("Pig");
 linkMe.cminsertNode("Away");
 linkMe.cmlistList();
 cout << endl;
 linkMe.cmremoveNode("The");
 linkMe.cmremoveNode("Cow");
 linkMe.cmremoveNode("Away");
 linkMe.cmlistList();
    return 0;
}

LinkedList code:

/*
 *  LinkedList.h
 *  Lab 6
 *
 *  Created by Anthony Glyadchenko on 3/22/10.
 *  Copyright 2010 __MyCompanyName__. All rights reserved.
 *
 */

#include <stdio.h>
#include <iostream>
#include <fstream>
#include <iomanip>

using namespace std;


class ctNode {

 friend class ctlinkList ; // friend class allowed to access private data

private:
 string sfileWord ;     // used to allocate and store input word
 int   iwordCnt   ;     // number of word occurrances
 ctNode* ctpnext    ;   // point of Type Node, points to next link list element
};


class ctlinkList {

private:
 ctNode* ctphead ;     // initialized by constructor 

public:
 ctlinkList () { ctphead = NULL ; }

 ctNode* gethead () { return ctphead ; }

 string cminsertNode (string svalue) {
  ctNode* ctptmpHead = ctphead ;
  if ( ctphead == NULL ) {                            // allocate new and set head
   ctptmpHead = ctphead = new ctNode ;
   ctphead -> ctpnext = NULL ;
   ctphead -> sfileWord = svalue ;
  } else {                                            //find last ctnode
   do {
    if ( ctptmpHead -> ctpnext != NULL ) ctptmpHead = ctptmpHead -> ctpnext ;
   } while ( ctptmpHead -> ctpnext != NULL ) ;     // fall thru found last node
   ctptmpHead -> ctpnext = new ctNode ;
   ctptmpHead = ctptmpHead -> ctpnext ;
   ctptmpHead -> ctpnext = NULL;
   ctptmpHead -> sfileWord = svalue ;
  }
  return ctptmpHead -> sfileWord ;
 }

 string cmreturnNode (string svalue) {
  return NULL;
 }

 string cmremoveNode (string svalue) {
  int counter = 0;
  ctNode *tmpHead = ctphead;
  if (ctphead == NULL) return NULL;
  while (tmpHead->sfileWord != svalue && tmpHead->ctpnext != NULL){
   tmpHead = tmpHead->ctpnext;
   counter++;
  }

  do{
   tmpHead->sfileWord = tmpHead->ctpnext->sfileWord;
   tmpHead = tmpHead->ctpnext;
  } while (tmpHead->ctpnext != NULL);

  return tmpHead->sfileWord;

 }

 string cmlistList () {
  string tempList;
  ctNode *tmpHead = ctphead;
  if (ctphead == NULL){
   return NULL;
  }
  else{
   while (tmpHead != NULL){
    cout << tmpHead->sfileWord << " ";
    tempList += tmpHead->sfileWord;
    tmpHead = tmpHead -> ctpnext;
   }
  }
  return tempList;
 }

};

Why is this happening?

Answer 1

At least one issue: the condition for your while loop is clearly wrong:

while (tmpHead->sfileWord != svalue || tmpHead->ctpnext != NULL)

It'll continue to move to the next item even if ctpnext is NULL but the sfileWord is not found, this will cause the tmpHead to become NULL in the next iteration (when it reaches the end of the linked list) and when it tries to fetch tmpHead->sfileWord, it'll fail.

It should be something like (depending on the code snippet after it):

while (tmpHead != NULL && tmpHead->sfileWord != svalue)

Answer 2

I think it should be an AND comparison instead of an OR

while (tmpHead->sfileWord != svalue && tmpHead->ctpnext != NULL){

otherwise tmpHead will eventually become NULL if the value is not found.