-
Notifications
You must be signed in to change notification settings - Fork 41
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
[6.0] New collection methods #1010
Conversation
…preter into i1004
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but please address the comments.
* @return the element at the given index | ||
* @throws ArrayIndexOutOfBoundsException if `i < 0` or `length <= i` | ||
*/ | ||
def get(i: Int): Option[A] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ScalaDoc doesn't correspond to the method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -76,6 +92,14 @@ trait Coll[@specialized A] { | |||
* produces a collection ((x0, y0), ..., (xK, yK)) where K = min(N, M) */ | |||
def zip[@specialized B](ys: Coll[B]): Coll[(A, B)] | |||
|
|||
/** For this collection (x0, ..., xN) and other collection (y0, ..., yM) | |||
* produces a collection ((x0, y0), ..., (xK, yK)) where K = min(N, M) */ | |||
def startsWith(ys: Coll[A]): Boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ScalaDoc is not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
/** For this collection (x0, ..., xN) and other collection (y0, ..., yM) | ||
* produces a collection ((x0, y0), ..., (xK, yK)) where K = min(N, M) */ | ||
def endsWith(ys: Coll[A]): Boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ScalaDoc is not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -350,6 +354,10 @@ class PairOfCols[@specialized L, @specialized R](val ls: Coll[L], val rs: Coll[R | |||
|
|||
def zip[@specialized B](ys: Coll[B]): PairColl[(L,R), B] = builder.pairColl(this, ys) | |||
|
|||
def startsWith(ys: Coll[(L, R)]): Boolean = toArray.startsWith(ys.toArray) | |||
|
|||
def endsWith(ys: Coll[(L, R)]): Boolean = toArray.endsWith(ys.toArray) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is simple, but inefficient ((ys.length + 1)*2 allocations) fallback implementation.
In the case when ys has type PairOfCols, it is possible to recursively call into ls
and rs
, and avoid allocations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, added tests also
// ======== 6.0 methods below =========== | ||
|
||
val ReverseMethod = SMethod(this, "reverse", | ||
SFunc(Array(ThisType), ThisType, paramIVSeq), 30, Zip_CostKind) // todo: costing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to declare the descriptor by just cloning from Append (Zip is too small).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
} | ||
|
||
val DistinctMethod = SMethod(this, "distinct", | ||
SFunc(Array(ThisType), ThisType, paramIVSeq), 31, Zip_CostKind) // todo: costing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cost of distinct
is larger then zip or Append, probably by a factor of 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Seq( | ||
Coll(1, 2) -> Expected(ExpectedResult(Success(Coll(1, 2)), None)), | ||
Coll(1, 1, 2) -> Expected(ExpectedResult(Success(Coll(1, 2)), None)), | ||
Coll(1, 2, 2) -> Expected(ExpectedResult(Success(Coll(1, 2)), None)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to add a case with unordered collections and many different duplicates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
(Coll(1, 2, 3), Coll(1, 2)) -> Expected(ExpectedResult(Success(true), None)), | ||
(Coll(1, 2, 3), Coll(1, 2, 3)) -> Expected(ExpectedResult(Success(true), None)), | ||
(Coll(1, 2, 3), Coll(1, 2, 3, 4)) -> Expected(ExpectedResult(Success(false), None)), | ||
(Coll[Int](), Coll[Int]()) -> Expected(ExpectedResult(Success(true), None)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not covered cases:
- Coll(1, 2, 3), Coll(1, 2, 4)
- Coll[Int](1, 2), CollInt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
In this PR, new collection methods got introduced:
close #1004