Adds append, appendAll and prepend to Buffer#6879
Conversation
| * @param elems the elements to append. | ||
| */ | ||
| @deprecated("Use addAll instead", "2.13.0") | ||
| def append(elems: A*) { appendAll(elems) } |
There was a problem hiding this comment.
I suggest to not use the procedure syntax (which has been deprecated), and to forward to addAll directly:
def append(elems: A*): Unit = addAll(elems)
def appendAll(xs: IterableOnce[A]): Unit = addAll(xs)There was a problem hiding this comment.
updated. thanks.
| */ | ||
| def prepend(elem: A): this.type | ||
|
|
||
| @`inline` final def append(elem: A): this.type = addOne(elem) |
There was a problem hiding this comment.
Why remove this one? Users will end up with the deprecated (and less efficient) version instead. I don't think it makes sense to deprecate append when prepend is still supported.
There was a problem hiding this comment.
Oh right, I forgot about that (see #6819 )…
So, do we want to have both append and appendAll, in addition to addOne and addAll, but for the sake of symmetry with prepend and prependAll?
| * @param elems the elements to append. | ||
| */ | ||
| @deprecated("Use addAll instead", "2.13.0") | ||
| def append(elems: A*): Unit = addAll(elems) |
There was a problem hiding this comment.
The deprecated methods should be @inline final. Otherwise they are prone to getting overridden (which may not have the intended semantics).
There was a problem hiding this comment.
What about making them @deprecatedOverriding instead of final to not break existing overrides?
There was a problem hiding this comment.
Added @deprecatedOverriding and @inline tag.
|
Hey @Prithvirajbilla, thanks a lot for your work. However, I gave you a wrong direction in the initial issue description and I’m sorry about that. I’ve explained in scala/bug#10970 (comment) what I think we should do instead. We appreciate a lot if you still want to fix the issue, just let us know! |
|
I'd like to fix the issue. have gone through your comments. I will send a PR this weekend. |
|
Updated:
|
| * | ||
| * @param elem the element to append. | ||
| */ | ||
| final def append(elem: A): this.type = addOne(elem) |
There was a problem hiding this comment.
You could keep the @inline annotation.
| /** Appends the elements contained in a iterable object to this buffer. | ||
| * @param xs the iterable object containing the elements to append. | ||
| */ | ||
| final def appendAll(xs: IterableOnce[A]): this.type = addAll(xs) |
|
this needs further work, see |
|
what do you recommend that we do here? revert overloaded |
Fixes scala/bug#10970
reference to append and appendAll methods in Buffer (scala 2.12.x)
https://github.com/scala/scala/blob/2.12.x/src/library/scala/collection/mutable/BufferLike.scala#L145