Bug or desired behavior: "contains?" doesn't seem to work on java.util.Sets.

[Jason Wolfe]

While "contains?" treats Clojure maps and java.util.Maps the same, it
doesn't seem to work on java.util.Sets:

user> (let [m (HashMap.)] (.put m "test" "test") (contains? m "test"))
true
user> (let [s (HashSet.)] (.add s "test") (contains? s "test"))
false
user> (let [s (HashSet.)] (.add s "test") (contains? (set s) "test"))
true

Bug or desired behavior?

Thanks,
Jason

[Jason Wolfe]

This just bit me a second time, since one of my revised set functions
uses "contains?" and thus doesn't work on java.util.Sets (or even
the .keySets of Clojure maps).

user> (contains? (.keySet {:a :b}) :a)
false

It seems that all that's required to make "contains?" work on general
Sets is to replace "IPersistentSet" with "Set" on lines 648 and 649 of
RT.java. I can make a patch if desired.

If the current behavior is as-desired, I would like to understand
why. Is it that some java.util.Sets have linear lookup time or
something like that?

Thanks,
Jason

[Stephen C. Gilardi]

On Feb 3, 2009, at 10:44 PM, Jason Wolfe wrote:

user> (contains? (.keySet {:a :b}) :a)
false

It seems that all that's required to make "contains?" work on general
Sets is to replace "IPersistentSet" with "Set" on lines 648 and 649 of
RT.java.  I can make a patch if desired.

Along those lines, would it be a good idea for the object returned by clojure.core/keys to implement java.util.Set and/or clojure.lang.IPersistentSet?

--Steve

[Jason Wolfe]

Yeah, that would be great.  (Or, at least, a separate "keyset" function).  I would ask for IPersistentSet, since java.util.Sets don't play as well with Clojure (e.g., you can't call "into" on them, and they aren't "set?"s).  

Thanks,

Jason

[Rich Hickey]

On Feb 3, 10:44 pm, Jason Wolfe <jawo...@berkeley.edu> wrote:
> This just bit me a second time, since one of my revised set functions
> uses "contains?" and thus doesn't work on java.util.Sets (or even
> the .keySets of Clojure maps).
>
> user> (contains? (.keySet {:a :b}) :a)
> false
>
> It seems that all that's required to make "contains?" work on general
> Sets is to replace "IPersistentSet" with "Set" on lines 648 and 649 of
> RT.java. I can make a patch if desired.
>

Patch welcome - must handle both overloads of RT.get() - thanks!

Rich

[Rich Hickey]

Right now keys is just a filter on the seq of the map. I wouldn't be
opposed to it being an IPersistentSet, but keys must be a constant-
time operation, so the set would have to be a view on the map, not a
copy of the keys.

Rich

[Jason Wolfe]

OK, thanks! I will take a shot at making patches for both of these.

-Jason

[Jason Wolfe]

Hah, I'm not sure if you are serious or trying to get me to answer my
own question ... either way, thanks :)

I went to implement the RT.get() portion of this patch, and was very
surprised to find that there seems to be no way to get "the [actual,
identical?] object that is held in the set which compares equal to the
key, if found" from a java.util.Set in sublinear time. You could
use .contains and return "an object .equal to the object in the set,
if found", but I suspect this is not what "get" is supposed to do. In
this case, I guess there is good reason that "contains?" does not work
on java.util.Sets :). Please correct me if I'm wrong on the contract
of "get"; otherwise, I'll drop this.

Thanks!
Jason

[Jason Wolfe]

OK, I put up an issue & patch here:

http://code.google.com/p/clojure/issues/detail?id=66&colspec=ID%20Type%20Status%20Priority%20Reporter%20Owner%20Summary

Comments/questions/criticisms welcome.

-Jason