zipper: issue with latest test case from sync

[SleeplessByte]

[Original Issue:]

The following test is missing:

# different paths to same zipper
"47aa85a0-5240-48a4-9f42-e2ac636710ea" = true

[sachinmk27]

I'd like to work on this

[joshgoebel]

@SleeplessByte I don't understand this test at all... It seems wholly at odds with the problem specificiation:

• prev (move the focus to the previous child of the same parent, returns a new zipper)
• next (move the focus to the next child of the same parent, returns a new zipper)
• up (move the focus to the parent, returns a new zipper)
• set_value (set the value of the focus node, returns a new zipper)

All mutation/navigation returns a new zipper, this directly contradicts the most recent test of "same zipper".

Should I open a new issue?

[joshgoebel]

To me I see 3 levels of abstraction here...

1. the underlying node data in memory
2. the JavaScript reference to a node (variable, object key, etc)
3. the Zipper instance (which holds unto a node reference)

I might have two zippers a and b that hold the same reference (2) to the same data(1) - because they are the same path (even if arrived at my different routes), but they are (in my implementation) different pointer instances (3)... I'm confused as to why this should be a requirement or even exactly what it means for it to be a requirement.

The directions say nothing about this requirement.

Questions:

• It seems that what matters here should be 1/2 (pointer equality) not 3 (whether we have 1 or 2 discrete pointers), no?
• And of course this stops being true if one calls a set anywhere along the route, correct?
• How does this test even pass given the code for right - it literally calls new Zipper() always...

[joshgoebel]

This also seems a direct contradiction of the immediate prior test:

  test('up returns a new Zipper', () => {
    const up = zipper.right().up();
    expect(zipper).not.toBe(up);
  });

These are the same path (root), so why are they not the "same" zipper?

[joshgoebel]

Wow... I still think there is a lot to be confused about here (and I'm conflating toBe and toEqual above), but I don't think the test names help add much clarity in this regard... "same zipper" in what sense? I'd personally word it more like "different paths lead to same node" (which is actually what is truly being compared, not zipper instances).

...but I did get the last test to pass easily by changing:

  value = () => this.node.value

(which seems to trip up equality since I assume there are now two different value functions)

to

    value() {
        return this.node.value
    }

Are we purposely intending to penalize students for writing code in this fashion?

---

Proposal:

test('should access same node via different paths', () => {
    const z1 = zipper.left().up().right();
    const z2 = zipper.right();
    expect(z1.node).toBe(z2.node);
});```

[junedev]

@joshgoebel I re-opened this and adjusted the title and labels, then you don't have to copy your thoughts into a new issue. I put "discussion" for now until a maintainer had time to take a closer look and decide how to proceed.