0

I am writing a linked list (not using Java's) and trying to create a get method to return an element of the list by its index number. Originally, I wrote this using a for loop. My code is repeatedly failing a test in which it retrieves the element at index 0 (I seem to be able to retrieve elements at other indices). curr is just for me to keep track of the current node.

public double get(int index) {
    Node curr = this.sentinel.next;

    for (int i = 0; i < this.size(); i++) {
        if (i == index) {
            return curr.data;
        }
        curr = curr.next;
        if (index > numElts) {
            return Double.NaN;
        }
        if (index < 0) {
            return Double.NaN;
        }
    }
    return Double.NaN;
}

I thought the for loop might be what was giving me trouble, so I wrote it as a while loop.

 while (curr != null) {
  if (i == index) {
   i++;
   return curr.data;
 }
 curr = curr.next;
 }

However, I'm still having trouble retrieving the element at the 0 index. I appreciate any input on how these methods of traversal might be problematic. I'm kind of lost. Also apologies if my formatting is off, still getting used to formatting on this site.

Caroline
  • 85
  • 7
  • Note that your bounds checks (e.g. `if (index > numElts) {`) should be before the loop, as they don't change value inside the loop body. – Andy Turner Sep 07 '17 at 22:56
  • @AndyTurner The internal bounds check shouldn't even be necessary because they are only iterating to `< this.size()` anyway. – billoot Sep 07 '17 at 22:58
  • @billie true, but why bother iterating the whole list if you know it's out of bounds to start with? – Andy Turner Sep 07 '17 at 22:59

1 Answers1

0

You're initializing curr as sentinal.next, wouldn't this cause you to skip over the first element? You should also have an off-by-one error for every element, as if your list was 1-indexed instead of 0-indexed.

In the while loop, you're not iterating i unless it's equal to the index, so you'll never find the case that i == index unless index is 0.

Bricky
  • 2,572
  • 14
  • 30
  • Sentinals are normally used as elements that sit before and after the first and last elements of a data structure, to simplify logic without having to use so many null checks. Assuming that part of it is set up correctly, the starting sentinel.next should be the 0 index element. – billoot Sep 07 '17 at 22:56
  • I think that's the only place the error could be, as the for loop would break and return on line 7 if `index` is 0. Unless I'm missing something. – Bricky Sep 07 '17 at 22:59
  • I think you're right, which makes me think it's an error in either setting up or interpreting the data. – billoot Sep 07 '17 at 23:01
  • @billie, yes the first sentinel.next should be referencing the element at index 0. I am under the impression that this is true of the code I have written (at the start of the for loop, curr refers to the element at index 0). Please let me if I'm wrong about that. Thank you! – Caroline Sep 08 '17 at 00:58
  • I also received this feedback about my for loop "You might consider starting your looping at the node that is sentinel.next rather than starting at sentinel. If length is 1 then sentinel.next is a real node, not null. Then you skip the sentinel and can count data cell nodes" I believe this is what I am doing--I start with Node curr = this.sentinel.next, which corresponds to index 0. That would skip the sentinel and only count nodes. So I'm pretty lost about what could be the issue – Caroline Sep 08 '17 at 01:01