Reputation: 11

searching in linked list

Here is my code for searching in a linked list, but it doesn't give me the correct result. Please help me, i'm very worried about it.

search() {

  char ser[20];
  cout << "enter data to be searched" << endl;
  gets(ser);

  for ( start=head; start->ptr!=NULL; start=start->ptr ) {
    if ( start->info == ser ) {
      cout << "ok" << endl;
      break;
    }
  }
  cout << "not found" << endl;
}

Thanks, Saima Kanwal Bhutta

Upvotes: 1

Answers (6)

Thomas Edleson

Reputation: 2196

Gets is dangerous, as it provides no way to specify the length of your buffer. There are alternatives to use with char arrays, but here it would simply be easier to use a std::string. I've extracted the find functionality into a separate function. This allows you to use the same function to search the list regardless of how you get the value to search for, or what you want to do with it.

Node* find(Node* head, const string& needle) {
    for (; head; head = head->ptr) {
        if (head->info == needle) {
            return head;
        }
    }
    return 0;
}

void search(Node* head) {
    string needle;
    cout << "Data to be searched: ";
    if (!getline(cin, needle)) {
        // Do something appropriate, such as throw an exception, return
        // an error code (if you change the function's interface), or
        // simply exit.
        abort();
    }

    Node* found = find(head, needle);
    if (found) {
        cout << "Found.\n";
    }
    else {
        cout << "Not found.\n";
    }
}

Upvotes: 0

Eric Z

Reputation: 14505

In your case, you should compare the contents of strings instead of the starting address of them.

Correct version:

void search() {

  char ser[20];
  cout << "enter data to be searched" << endl;
  gets(ser);

  for (start=head; start->ptr!=NULL; start=start->ptr)
  {
    if (strcmp(start->info, ser) == 0)
    {
      cout << "found" << endl;
      return;
    }
  }
  cout << "not found" << endl;
}

A bit more to mention

You need check head first before the for loop. Otherwise, the program will crash if head is NULL.

Upvotes: 0

corlettk

Reputation: 13574

Saima,

Firstly, Welcome to the forums, and welcome also the wonderful, frustrating, and fruitful world of computer programming.

Secondly, I edited your post. If you click the edit button now you'll see how to layout your source-code, so the forum displays it nicely.

Thirdly, I guess you meant return where you said break ... so that you don't allways see the "not found" message. Is that what you wanted?

Fourthly, I suggest you seperate the user-input part from the list-search part... it's easily done, and it makes the linked-list-search usable with any string (from anywhere), not just one which user enters right now. Likewise seperate the output from the search, that way you can re-use the search later, to produce whatever output is appropriate in the circumstances.

Lastly, Those variable names (forgive me) suck!

So... My ANSI-C version would look something like this:

int contains(char* target) {
  for ( Node node=head; node->next!=NULL; node=node->next ) {
    if ( strcmp(node->data, target)==0 ) {
      return 0; // TRUE
    }
  }
  return 1; // FALSE
}

The above are "fairly standard" names for the parts of a linked list, which help to make your code that much more readable, and therefore maintainable. Also WTF is a "ser"... how-about "target"?

If this is all over your head then don't worry about it... just ignore this advise, for now.

Cheers. Keith.

Upvotes: 1

asami

Reputation: 803

Your loop condition is dangerous. You are not checking if the 'start' itself is NULL or Not. Furthermore, you are comparing if the Next element is available or not and thus losing the current element if next element is not available. String comparison is also incorrect. Update your loop as following:

for ( start=head; start != NULL; start=start->ptr ) {
        if ( strcmp(start->info, ser) == 0 ) {
          cout << "ok" << endl;
          break;
        }
      }

Upvotes: 0