You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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
which would make more sense IMHO if it was
otherwise you lose the only advantage of having a default, e.g. when writing
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.The text was updated successfully, but these errors were encountered: