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

Questionable change of SequenceableCollection>>#splitOnFirst: #914

Open
riuttner opened this issue Oct 31, 2024 · 0 comments
Open

Questionable change of SequenceableCollection>>#splitOnFirst: #914

riuttner opened this issue Oct 31, 2024 · 0 comments

Comments

@riuttner
Copy link

I ran into an issue because I was using the old implementation which defaulted to nil, such that my #ifNil: check now never succeeded. The original code now has been extracted to

`^ self splitOnFirst: anObject noneValue: #()` 

which would make more sense IMHO if it was

`^ self splitOnFirst: anObject noneValue: self species new` 

otherwise you lose the only advantage of having a default, e.g. when writing

`(myString splitOnFirst: $:) second , ' something to add'`

In case of triggered default, the latest implementation would end up with an Array of Character instead of a String as a bad surprise for follow-up code. I think the literal Array has been chosen as default because it avoids creating a new instance of whatever that anyway will immediately be dropped, as most subclasses of SequenceableCollection cannot change their instance size - BUT: Even my made-up example does not look like a real-world use case taking a reasonable advantage of an empty collection as a default, and IMHO there will be no other convincing cases. In the Microdown package itself I only see 2 senders of the extended call, always passing nil, and my (yet private) Microdown extensions will add 2 further ones which also first have to check if something is there, where #ifNil: is sufficient for, too. Thus I strongly vote for returning to noneValue: nil as default. We could argue that 4 cases are not that important - OTOH I know a lot of cases where extensions brought in by widely used external modules will also be used outside of the original scope, thus we could avoid later bigger refactoring cycles as early as possible.

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

No branches or pull requests

1 participant