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

Add union to TreeMap #83

Merged
merged 4 commits into from
Jan 26, 2015
Merged

Add union to TreeMap #83

merged 4 commits into from
Jan 26, 2015

Conversation

dobesv
Copy link
Contributor

@dobesv dobesv commented Jan 15, 2015

No description provided.

}
return result;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets call this union. This is the defition of union in Haskell, https://downloads.haskell.org/~ghc/6.12.2/docs/html/libraries/containers-0.3.0.0/Data-Map.html#v%3Aunion:

union :: Ord k => Map k a -> Map k a -> Map k a 
O(n+m). The expression (union t1 t2) takes the left-biased union of t1 and t2. It prefers t1 when duplicate keys are encountered, i.e. (union == unionWith const). The implementation uses the efficient hedge-union algorithm. Hedge-union is more efficient on (bigset `union` smallset).

union (fromList [(5, "a"), (3, "b")]) (fromList [(5, "A"), (7, "C")]) == fromList [(3, "b"), (5, "a"), (7, "C")]

Please review and let me know when this is updated. Please update the javadoc, return type (it should return TreeMap<K, V>) and method body.

There is an opportunity to implement some of the Haskell combine methods: unionWith, unionWithKey, unions and unionsWith.

There are also lots of other methods we could add from the Haskell doc above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it left biased in that case?
On Jan 15, 2015 3:16 AM, "Mark Perry" notifications@github.com wrote:

In core/src/main/java/fj/data/TreeMap.java
#83 (diff)
:

  • /**
  • * Extend this TreeMap to include all mapping from another TreeMap. Where a
  • * mapping exists in both this TreeMap and the other one, the value from the
  • * other one will replace the value in this one.
  • * @param other
  • * @return
  • */
  • public Object setAll(TreeMap<K, V> other) {
  •   TreeMap<K, V> result = this;
    
  •   for(P2<K,V> p : other) {
    
  •       result = result.set(p._1(), p._2());
    
  •   }
    
  •   return result;
    
  • }

Lets call this union. This is the defition of union in Haskell,
https://downloads.haskell.org/~ghc/6.12.2/docs/html/libraries/containers-0.3.0.0/Data-Map.html#v%3Aunion
https://downloads.haskell.org/%7Eghc/6.12.2/docs/html/libraries/containers-0.3.0.0/Data-Map.html#v%3Aunion
:

union :: Ord k => Map k a -> Map k a -> Map k a
O(n+m). The expression (union t1 t2) takes the left-biased union of t1 and t2. It prefers t1 when duplicate keys are encountered, i.e. (union == unionWith const). The implementation uses the efficient hedge-union algorithm. Hedge-union is more efficient on (bigset union smallset).

union (fromList [(5, "a"), (3, "b")]) (fromList [(5, "A"), (7, "C")]) == fromList [(3, "b"), (5, "a"), (7, "C")]

Please review and let me know when this is updated. Please update the
javadoc, return type (it should return TreeMap) and method body.

There is an opportunity to implement some of the Haskell combine methods:
unionWith, unionWithKey, unions and unionsWith.

There are also lots of other methods we could add from the Haskell doc
above.


Reply to this email directly or view it on GitHub
https://github.com/functionaljava/functionaljava/pull/83/files#r23003389
.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, only add the key value pair if the key does not exist in the first map.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mperry mperry changed the title Add setAll() method to TreeMap, to combine two TreeMap instances. Add union to TreeMap Jan 16, 2015
@mperry
Copy link
Contributor

mperry commented Jan 21, 2015

@dobesv Any news of progress on this issue?

@dobesv
Copy link
Contributor Author

dobesv commented Jan 26, 2015

@mperry OK I pushed some changes, please take a look. I had a case where I wanted to just merge in a list of pairs, and I added that as well.

mperry added a commit that referenced this pull request Jan 26, 2015
@mperry mperry merged commit 01aef1b into functionaljava:master Jan 26, 2015
@mperry
Copy link
Contributor

mperry commented Jan 26, 2015

Thanks @dobesv for your contribution.

@mperry mperry added this to the v4.3 milestone Jan 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants