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 58 forks source link

`groupWhen` and `groupWhenWithAcc` proposal/design question #142

Open sebsheep opened 3 years ago

sebsheep commented 3 years ago

At work, I came to the need of some functions which I hoped to find in List.Extra. I'll expose what was my initial issue, and have some questions.

So here is the first function:

groupWhen : groupWhen : (a -> Bool) -> List a -> List (List a)

The initial need was to group a list a words with punctuation into sentences:

isFinalWord word = 
    case String.right 1 word of
          "!" -> True
          "." -> True
          "?" -> True
          _ -> False

groupWhen isFinalWord  ["Hello!", "I", "am", "Sébastien.", "I", "love", "elm!"]
       == [ ["Hello!"], [ "I", "am", "Sébastien."], ["I", "love", "elm!"]]

So when the predicate returns True, we close the "current" group including the current element.

So first wording question, I'm not very happy with the "group" name since it is already use in this lib for other thing. Thoughts?

Then I had to group those sentences in paragraphs such that the total length of each paragraph is maximum X chars long. So I created groupWhenWithAcc.

groupWhenWithAcc : (a -> acc -> ( Bool, acc )) -> acc -> List a -> List (List a)

groupWhenWithAcc
    (\ sentence charCount ->
           let newCount = String.length sentence + charCount in
           if newCount  < X then
                (False, newCount)
           else           
                (True, 0)
    ) 
    [ "Hello, I'am Sébastien.", "I love elm.", "Thiiiiiiiis iiiiiiis a verrrrrrrrrrrry loooooooong sentenceeeeeee.", "Bye."]

And here I was a bit uncomfortable: the "long sentence" should have its own paragraph, but it was not possible with the current design, the splitWhenWithAcc would either:

I went into the conclusion that the predicate should return something more elaborated than a bool -- my function doesn't have to decide whether include the current element in the current group or not. This is my wording question: what should be the name of the type and following variants:

type GroupWhenResponse
   = ContinueAndDontCreateANewGroup
   | FinishTheGroupIncludingTheCurrentElement
   | FinishTheGroupWithoutTheCurrentElementAndPutTheCurrentInASingleton
   | FinishTheGroupWithoutTheCurrentElementAndStartANewGroupWithCurrentElement

My function became so:

groupWhenWithAcc : (a -> acc -> ( GroupWhenResponse, acc )) -> acc -> List a -> List (List a)

groupWhenWithAcc
    (\ sentence charCount ->
           if String.length sentence > X then
              ( FinishTheGroupWithoutTheCurrentElementAndPutTheCurrentInASingleton, 0)
          else
                let newCount = String.length sentence + charCount in
                if newCount  < X then
                    (ContinueAndDontCreateANewGroup, newCount)
                else           
                    (FinishTheGroupWithoutTheCurrentElementAndStartANewGroupWithCurrentElement, 0)
    ) 
    [ "Hello, I'am Sébastien.", "I love elm.", "Thiiiiiiiis iiiiiiis a verrrrrrrrrrrry loooooooong sentenceeeeeee.", "Bye."]

The FinishTheGroupIncludingTheCurrentElement variant being used in my first function:

isFinalWord word = 
    case String.right 1 word of
          "!" -> FinishTheGroupIncludingTheCurrentElement
          "." -> FinishTheGroupIncludingTheCurrentElement
          "?" -> FinishTheGroupIncludingTheCurrentElement
          _ -> ContinueAndDontCreateANewGroup

groupWhen isFinalWord  ["Hello!", "I", "am", "Sébastien.", "I", "love", "elm!"]
       == [ ["Hello!"], [ "I", "am", "Sébastien."], ["I", "love", "elm!"]]

So what do you think about all of that? Ideas for wording? Am I going too far in customization?

Chadtech commented 2 years ago

I think having a type with these very particular constructors adds a kind of complexity to the function that I think makes it harder to use in more general use cases.

Maybe having a function like (a -> acc -> (Bool, acc)) has some usefulness. I am not sure what the real world use case would be for that, but that function seems to have properties that dont exist in the current List.Extra api.