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

Implement flatMapValues method #1348

Merged
merged 4 commits into from
Jul 1, 2015
Merged

Conversation

danosipov
Copy link
Contributor

Sometimes its useful to have flatMapValues, similar to mapValues, but flatter.

* Similar to mapValues, but works like flatMap, returning a collection of outputs
* for each value input.
*/
def flatMapValues[V](fn: T => GenTraversableOnce[V]): This[K, V] =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we generally use TraversableOnce[V] here, can we stick to that style?

@johnynek
Copy link
Collaborator

johnynek commented Jul 1, 2015

can you add the same method to TypedPipe? we try to add all methods that make sense on both to both. Something like:

def flatMapValues[K, V1, V2](fn: V1 => TraversableOnce[V2])(implicit ev: T <:< (K, V1)): TypedPipe[(K, V2)] =
  raiseTo[K, V1].flatMap { case (k, v1) => fn(v1).map { v2 => k -> v2 } } 

@johnynek
Copy link
Collaborator

johnynek commented Jul 1, 2015

Thanks for adding this. You're right, it comes up, and summingbird added it a long time ago.

@danosipov
Copy link
Contributor Author

@johnynek Added the method to TypedPipe, but there appear to be no tests for it. It type checks though :)

EDIT: didn't see you provided an example - let me modify
EDIT2: Looks like they're equivalent - let me know if you disagree

@@ -300,6 +302,10 @@ trait TypedPipe[+T] extends Serializable {
def mapValues[K, V, U](f: V => U)(implicit ev: T <:< (K, V)): TypedPipe[(K, U)] =
raiseTo[(K, V)].map { case (k, v) => (k, f(v)) }

/** Similar to mapValues, but allows to return a collection of outputs for each input value */
def flatMapValues[K, V, U](f: V => TraversableOnce[U])(implicit ev: T <:< (K, V)): TypedPipe[(K, TraversableOnce[U])] =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want TypedPipe[(K, U)] to be the return type. In other words, you have another copy of mapValues here, which you can use if you don't want the flattening, but I think this one should behave exactly as it does on KeyedListLike.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if I use fn(v1).map { v2 => k -> v2 } This returns a Traversable[U] (or Traversable[V2] from example above)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look at my code. It had a flatMap yours does not have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😵 Of course, fixed

johnynek added a commit that referenced this pull request Jul 1, 2015
@johnynek johnynek merged commit f76e5e2 into twitter:develop Jul 1, 2015
@johnynek
Copy link
Collaborator

johnynek commented Jul 1, 2015

Thanks! This is useful.

@ianoc ianoc mentioned this pull request Aug 10, 2015
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

Successfully merging this pull request may close these issues.

2 participants