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

lists:concat/1 considered dangerous? #7068

Open
TD5 opened this issue Mar 28, 2023 · 0 comments
Open

lists:concat/1 considered dangerous? #7068

TD5 opened this issue Mar 28, 2023 · 0 comments
Assignees
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@TD5
Copy link
Contributor

TD5 commented Mar 28, 2023

Describe the issue

lists:concat/1 has surprising semantics for most new users, in my experience.

I think most people see lists:concat/1, and presume it is about creating a big list from one or more smaller lists. In fact, in that situation most people probably was lists:append or lists:flatten, but erroneously use lists:concat because, going by the name, it sounds like would do something different.

Here are the docs for the lists:concat/1 function:

concat(Things) -> string()
    Types
        Things = [Thing]
        Thing = atom() | integer() | float() | string()
        
Concatenates the text representation of the elements of Things. The elements of Things can be atoms, integers, floats, or strings.

In some sense, the function name isn't wrong: It really does take a series of things which have a fairly natural string representation, and ties them together in series into one big list-of-characters. Unfortunately, if you meant to just directly chain a series of general purpose lists together (à la lists:append), instead of getting an error at runtime if you make a mistake with the inputs to the function, you can get a sort-of-what-you-wanted result, which can make for tricky debugging.

Expected behavior

I think most people would agree it be a bad idea to change the meaning of lists:concat/1 now, and I know of many uses of it that use it as intended.

My proposal would be to officially deprecate lists:concat/1 under that name, and rename it to something that's less easily confused with lists:append / lists:flatten. That way, there would be more friction to introducing new usages of lists:concat/1 without inspecting its meaning more closely.

Since the function seems to be primarily about pretty printing to a string, rather than a general purpose list operation, I also suspect it would make more sense for the logic to live in the string module. I don't really mind what the new name would be beyond that, but for the sake of discussion, I propose string:show/1. I am sure others can make better counter-proposals.

@TD5 TD5 added the bug Issue is reported as a bug label Mar 28, 2023
@rickard-green rickard-green added the team:VM Assigned to OTP team VM label Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

3 participants