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 HashMap.map, fix HashSet.equals #505

Merged
merged 3 commits into from
Aug 23, 2015

Conversation

patrox
Copy link
Contributor

@patrox patrox commented Aug 23, 2015

As a "side effect" of running tests for HashMap.map,
a problem with HashSet.equals was identified and fixed.

Fixes #503 and #504.

@Override
public <U> Set<U> map(Function<? super Entry<K, V>, ? extends U> mapper) {
return null;
Objects.requireNonNull(mapper, "mapper is null");
return foldLeft(HashSet.empty(), (acc, entry) -> acc.add(mapper.apply(entry)));
Copy link
Contributor

Choose a reason for hiding this comment

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

this is nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took the functional route to do it. I think that mr. Odersky would be satisfied ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@patrox
Copy link
Contributor Author

patrox commented Aug 23, 2015

Regarding HashSet.equals - yes, I agree that my analysis was a bit shallow - the most important is that it will be fixed!

OK, so I will close this one, as HashMap.map will be implemented in a separate PR.
Bottom line is that implementing Euler project problems (using javaslang) is a very good way to test it very thoroughly - it's a bit like an integration test - as we're mixing various types (Streams, Maps, Sets, CharSeqs) with various methods (i.e. maps, filters, groupBys).

@danieldietrich
Copy link
Contributor

Yes, the Euler tests are great for testing Javaslang.

Please implement the equals method in this PR I suggested above. I will pull it then. HashMap needs no separate PR.

Thx!

@danieldietrich
Copy link
Contributor

I will take care now of flatMap and map in general as described here: #488 (comment)

I can pull this into master today...

patrox added 3 commits August 23, 2015 23:22
If you execute a Stream.groupBy, followed by Stream.map,
which tries to access the entry.{key,value} it will crash with an NPE.
As a "side effect" of running tests for `HashMap.map`,
a problem with `HashSet.equals` was identified and fixed.
Thanks to @danieldietrich, HashSet.equals was further improved,
eliminating the risk of ClassCastException.
@patrox patrox force-pushed the stream_groupby_map_fix branch from 62beeca to f140aff Compare August 23, 2015 21:23
@patrox
Copy link
Contributor Author

patrox commented Aug 23, 2015

Done, coverage increased by 0.2% :)
But I'm still wondering how the order of elements in HashArrayMappedTrie could affect the HashSets from my 'minimal' example - containing 1 element ?

danieldietrich added a commit that referenced this pull request Aug 23, 2015
Implement HashMap.map, fix HashSet.equals
@danieldietrich danieldietrich merged commit 938a642 into vavr-io:master Aug 23, 2015
@danieldietrich
Copy link
Contributor

Many thanks :)

For your minimal example all should be ok. Currently it is not clear why the Iterator.equals failed - we will further investigate. The order of similar elements may change in general (having more than one element) because the Map (or HAMT) has a key set with no guarantee of order.

@danieldietrich danieldietrich modified the milestones: 2.0.3, 2.0.0 Aug 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NPE thrown while combining Stream.groupBy with Map.map
2 participants