Add groupBy to List#64
Conversation
|
@MartinSeeler, thanks for your interest in the project and your contribution. This branch compiles for 1.8.0_25, but not earlier versions (e.g. 1.8.0_05). I am not sure about the other versions between 5 and 25 (i.e. 1.8.0_11 and 20). This is due to calls to HashMap.foreach being ambiguous. I created issue #66 to deal with this, we will either need to fix it or workaround the issue. I think we should use the immutable map, TreeMap, instead of the mutable HashMap for these functions. I believe the mutable map is used internal purposes in places, but not exposed. I'd like to see a method with the very basic signature: Please use lambdas in your code (rather than anonymous inner classes). The old code has not been updated to use them, but we should be using them in new code. I see you wrote some demos, great. Can we add some unit tests or convert the demo to unit tests? Please add in core/src/test. If you are feeling adventurous you might like to try adding some property based tests in either core or tests (using scalacheck), but this is not necessary. Unit tests will be fine. Thanks again and I look forward to an updated pull request soon. |
|
Fixed #66 so that |
774ce40 to
94195af
Compare
|
Hi @mperry , I felt adventurous and wrote some property based tests in scala ;-) The I also switched from the inner anonymous classes to lambdas. I was not sure when I wrote the code, if it will still compile, and just adopted your style. Best regards, Martin |
|
@MartinSeeler, I fixed a problem you introduced with the immutable map in that you were adding an item to the map, but returning the old map, so no items were ever added. You can see that no items are output with the demo code. I also added a boolean to the scalacheck tests to ensure that the size is correct. I manually merged this into master by fetching your fork with a new remote, making my changes on a local branch and then merging into the trunk. |
I missed the functionality to easily group the elements of a given list.
Since I'm a beginner in functional programming, please let me know if this is already implemented in another way and unnecessary.
Cheers, Martin