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

Set.concat Not working as expected on String's #1910

Closed
rodhoward opened this issue Sep 4, 2022 · 3 comments · Fixed by #1913
Closed

Set.concat Not working as expected on String's #1910

rodhoward opened this issue Sep 4, 2022 · 3 comments · Fixed by #1913

Comments

@rodhoward
Copy link

When concatenating a new string Item into an existing set its treating the string as an array where as [].concat treats it as an item.

Selection_051

I would expect and prefer it to treat the string as an item.

Regards
Rod

@jdeniau
Copy link
Member

jdeniau commented Sep 5, 2022

You are right that this is inconsistent.

For the record, I suggest you to use .add instead of .concat, you will have the expected result.

The concat method takes one or several iterables as argument. As a string is an iterable of char, it does work.
At first sight, technically, the Set implementation seems right and the List wrong here.

@rodhoward
Copy link
Author

I think because javascript's array concat treats a string as a single item. e.g.

["one", "two"].concat("three")
(3) ['one', 'two', 'three']

Your better off following the list behavour in Set as well because its more intuitive.

@jdeniau
Copy link
Member

jdeniau commented Sep 5, 2022

You are right, and the immutable doc does explain this too !

List.concat
Returns a new List with other values or collections concatenated to this one.

Set
Returns a new Collection with other collections concatenated to this one.

So the List does handle passing values. We might update Set to be consistant with Javascript implementation

The line that test it is this line :

If argument has iterator and is not a string, so take it, instead do convert it into an array

typeof argument !== 'string' && hasIterator(argument)

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 a pull request may close this issue.

2 participants