Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/21740#discussion_r202545589
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/recommendation/MatrixFactorizationModel.scala
---
@@ -75,11 +75,29 @@ class MatrixFactorizationModel @Since("0.8.0") (
}
}
+ /** Validates the user and return the feature corresponding to the user.
*/
+ private def validateAndGetUser(user: Int): Array[Double] = {
+ userFeatures.lookup(user) match {
+ case userFeature: Seq[Array[Double]] =>
--- End diff --
Why the pattern match? `.lookup` should already be known to return a `Seq`
of the type of the RDD's values which is already known to be `Array[Double]`.
The name of the method isn't really accurate either; it gets a user vector,
not user.
```
val vec = userFeatures.lookup(user)
require(vec.nonEmpty, ...)
vec.head
```
How about one method instead of two, that takes the RDD as an arg? might
cut down on the code expansion here. I'm on the fence about whether this
warrants a new method since there are only two call sites, but it's reasonable.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
