Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds List.push and prepend #70

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Adds List.push and prepend #70

wants to merge 3 commits into from

Conversation

gampleman
Copy link
Collaborator

These come up as missing in pipelines quite often. I see a lot of code like

tags =
     node.tags
          |> Maybe.withDefault []
          |> (\tags -> tags ++ [ newTag.label ])
          |> Just

which could be a lot nicer with

tags =
     node.tags
          |> Maybe.withDefault []
          |> List.Extra.push newTag.label
          |> Just

Copy link
Collaborator

@lue-bird lue-bird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the prepend name confusing as hell in pipelines (what it's supposed to be used in)

[ 0 ] |> List.prepend [ 1 ] --> [ 0, 1 ]
-- "Wait I though it would prepend 1?

I would strongly suggest to instead introduce two functions prependTo and appendTo where the "to" signals it's use in a pipeline and what the "base list" is.

[ 0 ] |> prependTo [ 1 ] --> [ 0, 1 ]
[ 0 ] |> appendTo [ 1 ] --> [ 1, 0 ]

(used by minibill's elm-rope) but I'd also be fine with names like pushList-consList or appendSuffix-appendPrefix and similar.

@gampleman
Copy link
Collaborator Author

Hm, fair point @lue-bird. Although I don't really like functions that are only pipeline optimized. Perhaps we could do something with the l and r suffix? Like appendl and appendr?

@lue-bird
Copy link
Collaborator

lue-bird commented Feb 18, 2025

I don't think they are "only pipeline optimized".
E.g. remainderBy 3 x is still much more obvious than remainder x 3.
So I'd rather write prependTo [ 1 ] [ 0 ] than prepend [ 1 ] [ 0 ] (even though I can't really see many reasons to use prepend rather than append if you already call the function with both lists). Do you agree?

-l/-r IMO have the same problem as foldl/-r etc, it takes a bit of mental overhead to decipher and still is kind of ambiguous which is exactly the kind of issue a more rarely used helper shouldn't have

@gampleman
Copy link
Collaborator Author

@lue-bird ok, I've implemented the above. Are we happy for this to go in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants