elm-community / list-extra

Convenience functions for working with List.
http://package.elm-lang.org/packages/elm-community/list-extra/latest
MIT License
135 stars 59 forks source link

adding cycle and zipWithCycle function #72

Closed mrkaspa closed 6 years ago

mrkaspa commented 7 years ago

I added two new functions

Chadtech commented 7 years ago

Could you elaborate on a situation in which you would need these functions? I am guessing you have used these functions in your own projects? For what purpose did you use them?

mrkaspa commented 7 years ago

@Chadtech yes I needed to mix a list of colors with a list of elements, but there were three colors and a list of n elements so I wanted to repeat the list of colors and in Elm there's no streams or generators to do this

Chadtech commented 7 years ago

Sorry I dont understand. You cant mix two different types, and it sounds like you are saying you are mixing something like Color and Html msg. I am assuming I dont understand you.

And what do you mean by "mix"? Like interspersing?

mrkaspa commented 7 years ago

@Chadtech yes it's similar, but interwave doesn't create the tuple, the normal zip do it just until one list runs out of elements, in elixir for example I have this https://hexdocs.pm/elixir/Stream.html#cycle/1 but in elm I can't have lazy collections so zipWithCycle makes the second list as big as the first list to complete the zip

interweave [1,3,5,7,8] [2,4] == [1,2,3,4,5,7,8]
zipWithCycle [1,3,5,7,8] [2,4] == [(1,2), (3,4), (5,2), (7,2), (8,4)]
pzp1997 commented 7 years ago

Perhaps zipWithCycle should be called greedyZip (in keeping with the naming theme of greedyGroupsOf. And of course, this begs the question of whether there should be greedyMap2, greedyMap3, ... as well.

As for cycle, I am not sure if the interpretation of the integer argument is fully intuitive. I would either make it the number of times that the sequence should be repeated, i.e. cycle 3 [1, 2] == [1, 2, 1, 2, 1, 2] or exactly the length of the return list, i.e. cycle 4 [1, 2, 3] == [1, 2, 3, 1].

mrkaspa commented 7 years ago

@pzp1997 You're right, perhaps the names aren't the best, plz check the pr I updated it

Chadtech commented 7 years ago

Whats your actual use case tho? What are you using cycle for? Could you talk about the project a bit?

mrkaspa commented 7 years ago

@Chadtech I use it here https://github.com/mrkaspa/GitRankWeb/blob/master/src/Views.elm#L150 I use it to mix a tuple of colors with the data of a programming language here you can check it in action http://gitrank.mrkaspa.com/

Chadtech commented 7 years ago

Hmm. Okay I see what you are up to.

I wish I was able to think of a better way to do this, but I cant. I would really like to hear other opinions on this. I'll leave this PR open for a while, and maybe look to see if similar functions exist in other programming languages.

Regarding the names, repeat is already used in the core list package and does something different, so we should name it something else. Of the names thrown around so far I think the original ones were clearest cycle and zipWithCycle. Other ideas: concatSelf, cycleZip, and zipModulo.

Chadtech commented 7 years ago

In Haskell world theres a function called cycle that does largely the same thing. So I suggest we stick with that moving forward.

pzp1997 commented 7 years ago

I've been thinking about this PR, and here's what I've come up with. I think that the first argument to cycle should be the length of the resulting list. This gives people the most flexibility because they can have "partial" cycles if they want, i.e. cycle 4 [1, 2, 3] == [1, 2, 3, 1].

Regarding zipWithCycle, I do not think it should be included in the library. Given the version of cycle I described above, it is trivial to implement. I also think that its use is sufficiently rare that it's fine if people implement it themselves when they need it. In fact, I think that the use case @mrkaspa linked to above would be better suited by using cycle with map2.

Chadtech commented 6 years ago

@pzp1997 implemented cycle so I am closing this PR.