+1  A: 

That should work just fine. Have you tried it yet?

Marc W
yes and no. it's in my code but there are other parts missing and I haven't worked on it for a few days so not everything is "hooked up" yet
TheGNUGuy
+3  A: 

You might want to make sure that prevNode->link is not a null reference either, in case currentNode is not actually linked.

Jeff B
If currentNode is guaranteed to be in the list (which from the description it seems to be), the loop will always terminate before the end of the list.
sepp2k
A: 

You can even have:

while (prevNode && prevNode != currentNode)
    prevNode = prevNode->link;

But what you have looks fine.

Ashwin
+1  A: 

The code looks fine, but I would suggest a minor change to your while condition

while(prevNode != currentNode && prevNode != NULL)

For two reasons

  • Your code, as currently stated, could stop if the node we are looking for is pointed to by either prevNode or prevNode->link (and therefore we will have no idea which particular one of the two points to currentNode -- if we wanted to know, we would have to check with an if condition). With the change above, the target node is guaranteed to be stored in prevNode (if at all -- see next point).
  • For safety's sake, it would be good to check that prevNode is not NULL. However, as Pavel mentions, this test is unnecessary if currentNode is guaranteed to be in the list.


Edit in response to comment

Given that you don't need to know whether currentNode is in prevNode or prevNode->link, and since you want to stop (if possible) on currentNode == prevNode->link, then your original while is fine. However...

there is an if statement higher up in the code that prevents prevNode from being null already

It seems like you're missing the point of why you should check for NULL. Yes, that's good you check it before, but the reason why we have the NULL check in the loop is in the case where currentNode is not in the list, so you eventually reach the last node. Presumably (if you do this like most other linked lists) the value of link for your last node is NULL. If so, your current code will eventually end up calling NULL->link which of course will crash your program. That's why you should still check for NULL

while(prevNode != NULL && prevNode != currentNode && prevNode->link!=currentNode)

If you're absolutely sure that currentNode will be in the list, then I guess that check is also unnecessary, but it really is a good habit to get into.

GRB
The reason I have the condition that way because there is an if statement higher up in the code that prevents <code>prevNode</code> from being null already so basically I want to make sure that <code>prevNode</code> is equal to current Node or the node before <code>currentNode</code> which ever is first.
TheGNUGuy
+1  A: 

That code will traverse some part of your list, but which part depends on which way your list is linked. If it goes from head->tail (read: head node links to a node which links towards tail) then you would traverse your list starting from the random location to the tail. If the links are head<-tail then you would traverse from the random location to the head. In either case, you would not touch all the nodes in the list.

All of the above assumes some link list as such:

[head]<->[0...N Nodes]<->[Tail]

The links could be either way.

Also, you could link the head and tail nodes and create a circularly linked list. In that scenario, it is possible to visit all nodes by simply traversing until you are back to your original node.

Phage
A: 

A small thing I'd change with the code as posted (aside from the possibility discussed in other answers of running off the end of the list if currentNode isn't on the list or other error handling) is that when the while loop is done you don't know if prevNode or prevNode->link points to currentNode. This isn't a huge problem (since you can easily test for it), but it seems to me that it's best to test for this special case situation before the search, so it's clear that it's a special case.

Michael Burr
+1  A: 

Make sure you consider all of the possible edge cases:

  • What happens randomNode equals firstNode? What do you return? What should you return?
  • What happens when randomNode is the last node in the list?
  • What happens when randomNode is NULL?
  • What happens when randomNode is not in the list?

Now, not all of these cases may be applicable, depending on if you know anything about randomNode, and some of these may be trivial. But they are all worth thinking about.

If you consider all of these very carefully, there is a simple and elegant solution that handles them all.

Adam Rosenfield