nonEmptyTraverse for NEM with key mapping - #4759
#4759nonEmptyTraverse for NEM with key mapping#4759dpevunov-cp wants to merge 3 commits into
Conversation
|
Hi @dpevunov-cp , thank you for the PR! I think I understand what you're trying to achieve, but I've got a couple of suggestions about the proposed method:
|
| if (t.isEmpty) | ||
| Eval.now(G.map(f.tupled(h))(b => NonEmptyMap(b, SortedMap.empty[L, B](ordL.toOrdering)))) | ||
| else | ||
| G.map2Eval(f.tupled(h), Eval.defer(loop(t.head, t.tail)))((lb, acc) => NonEmptyMap(lb, acc.toSortedMap)) |
There was a problem hiding this comment.
Why wrap and unwrap with NonEmptyMap on every step? Why not do it once at the end after calling .value?
| loop(head, tail).value | ||
| } | ||
|
|
||
| def nonEmptyTraverse[G[_], L, B]( |
There was a problem hiding this comment.
I think this name isn't right. Traverse on maps only uses the values, not the keys. So if you are going to use the keys I think we need a new name. Something like nonEmptyTraversePairs or traverseWithKey or something.
There was a problem hiding this comment.
NonEmptyMap already has map (values only) and mapBoth (keys with values). So I guess this should be nonEmptyTraverseBoth then.
|
@dpevunov-cp , thank you for you commitment to the task! I'd really appreciate if you could add one or more tests to NonEmptyMapSuite for that new method. |
No description provided.