Implementation of Zipper should be private

[wernerdegroot]

I'd rather see the implementation of Zipper private.

Tests should be in terms of the List that the Zipper produces, instead of in terms of (what I feel is) an implementation detail.

This way we could change the way the Zipper works internally, without breaking the interface.

[stoeffel]

@joneshf has some great thoughts on this. See twitter thread https://twitter.com/st58/status/785507064910983169.
I totally agree with him, that Zipper should not be private and that we should possibly change Zipper a to type alias Zipper before current after = { before : List before, current : current, after : List after }. Will write something more in detail later today. When I digested all his thoughts (or maybe Hardy will comment here).

[wernerdegroot]

Basically, what I'm afraid of exposing is the fact that 'before' is reversed. This is something a user might miss when constructing a 'Zipper' themselves.

[wernerdegroot]

Other than that I think a parameterized type 'Zipper b c a' is great idea :)

[rtfeldman]

I will happily take the opposite position:

Opaque type Zipper a is a better API than exposed type alias Zipper before current after.

Let's discuss! 😄

[rtfeldman]

Basically, what I'm afraid of exposing is the fact that 'before' is reversed. This is something a user might miss when constructing a 'Zipper' themselves.

This is a great argument for a hidden implementation. This is a concern today, but the future concern is even greater.

Suppose you initially implemented it without reversing, because you weren't aware that optimization existed. (I know I wasn't, the first time I heard of zippers.) So you release the library, then later find out about the optimization, and release an update to the library which starts doing reversing.

In a world where type is exposed rather than type alias, this is no problem. The library says "here are a set of functions that work well together," and those are what people use. If you want to change an internal implementation detail like a list being reversed, you do so for all of those functions, and publish an upgrade. Everyone's code still works.

In the type alias world, anyone who was relying on the type alias to do something slightly different than the normal API is now in huge trouble. Their code is going to break in incredibly surprising ways, and there is no way for the type system to help them. The types haven't changed!

At this point the way to cause the least user pain is probably to abandon the current library, publish a fork called FastZipper, and tell people to use that instead. Hopefully this is the last time you discover such a chance to make a breaking improvement to the internal implementation, because once you get past SuperFastZipper and SuperDuperFastZipper this starts getting wordy. 😉

tl;dr Given the choice between "should I add unreliable support for hypothetical use cases" or "should I make this exact API as reliable as possible long-term," I think the latter is best. 🙂

[stoeffel]

@rtfeldman @wernerdegroot arg, sorry I didn't mean type alias Zipper before current after = { before : List before, current : current, after : List after } 😦
I meant type Zipper before current after = Zipper (List before) current (List after)

[stoeffel]

still, has the same concern with the reversed before.

[stoeffel]

You can run into the same problem with type Zipper a = Zipper (List a) a (List a) though.

zipperMap : (a -> b) -> List.Zipper.Zipper a -> List.Zipper.Zipper b
zipperMap transform zipList =
    List.Zipper.Zipper
        (List.foldl (\elem list -> transform elem :: list) [] (List.Zipper.before zipList))
-- ^==^ we expect before to be reversed here
        (transform (List.Zipper.current zipList))
        (List.map transform (List.Zipper.after zipList))

and by making it private, we make it harder for the user to implement such a function.

[stoeffel]

It's about me having a use case you didn't plan for.
-- @joneshf

[wernerdegroot]

But you can access the relevant values (before, current, after) through the accessors, right? Only these can offer the guarantee that elements are in the same order as in the original list, making the reversal of 'before' an implementation detail.

[scottcorgan]

@stoeffel Shouldn't zipperMap just go into a PR for the repo?

[stoeffel]

It's already in work by @wernerdegroot. But it's just an example for a use-case a user had. There could be other use-cases.

[stoeffel]

But you can access the relevant values (before, current, after) through the accessors, right? Only these can offer the guarantee that elements are in the same order as in the original list, making the reversal of 'before' an implementation detail.

true. But when we construct again with Zipper it's expected to be reversed.

[stoeffel]

@scottcorgan see #9

[rtfeldman]

But it's just an example for a use-case a user had. There could be other use-cases.

This will always be true though. There are infinitely many possible use cases, so it is necessarily impossible to anticipate them all.

The question is where to draw the line. As library authors, eventually we must say one of two things:

• "Nobody has told us they need this, but we will support it anyway just in case"
• "Nobody has told us they need this, and therefore we won't support it"

Essentially the former optimizes for the short-term and the latter optimizes for the long-term.

In the short-term, the more you expose, the more quickly someone can unblock themselves. This is true. The alternative would be that they would file an issue or create a fork, both of which would take more time.

In the long-term, the less you expose, the better experience you can provide for everyone, including people whose use cases were not initially anticipated.

If someone raises a use case that you did not anticipate at first, you can update the library to do a good job supporting that use case. You can also continue supporting it well for future releases, whereas you cannot help them if they resort to exposed-but-not-officially-supported APIs. The long-term maintenance burden has been shifted to the user.

Overall, I think Elm library authors should optimize for the long-term user experience, which means defaulting to exposing less. Even as someone who tends toward unusual use cases, as a library user I've been much happier with that experience (in, say, Elm's core libraries) than I have with other systems that were quicker to expose things. 😄

[artisonian]

I use a simpler implementation for my use case (maintaining a selection over a list where an element always has focus).

https://gist.github.com/artisonian/102b278ef1f6ed9fe851c448689dc2e9

[stoeffel]

. Overall, I think Elm library authors should optimize for the long-term user experience, which means defaulting to exposing less.

True!

The alternative would be that they would file an issue or create a fork, both of which would take more time.

Having more contributions is great and @wernerdegroot was always really responsive in the past ❤️

I'm open to give this a try.

[wernerdegroot]

Zipper is private since #26