-3

I am trying to test whether two HashSets of Strings contain identical Strings. The retainAll() method of Java Sets (which, as I understand it, implements the Collection interface) is a good way to check the intersection of two Sets. However, this method seems to test for equality using the == style check for whether they are references to the same memory object, rather than using the String's equals() method to check whether the contents are the same. Is there a way to get something the works like retainAll but that uses the equals() method?

I am trying to write code that checks whether a String contains a substring over a certain length from a certain other String. My strategy was to create a HashSet of each String containing all substrings of that length, then check whether the Sets contain Strings in common.

My current solution was to create my own static method that does what I want the retainAll method to do.

static boolean containsEqualElement(Set SetOne, Set SetTwo) {
    Iterator it = SetOne.iterator();
    while (it.hasNext()) {
        Object thisComp = it.next();
        Iterator it2 = SetTwo.iterator();
        while (it2.hasNext()) {
            if (it2.next().equals(thisComp)) {
                return true;
            }
        }
    }
    return false;
}

I'm not sure how the efficiency of this method compares to the retainAll method.

Todd R
  • 21
  • 1
  • 7
  • 5
    *this method seems to test for equality using the == style check*: no, it doesn't. It uses equals(). – JB Nizet Jan 28 '18 at 16:25
  • 1
    It definitely uses `equals`. Did you forget to implement `hashCode`? – Joe C Jan 28 '18 at 16:30
  • My current solution looks like this: static boolean containsEqualElement(Set SetOne, Set SetTwo) { Iterator it = SetOne.iterator(); while (it.hasNext()) { Object thisComp = it.next(); Iterator it2 = SetTwo.iterator(); while (it2.hasNext()) { if (it2.next().equals(thisComp)) { return true; } } } return false; } – Todd R Jan 28 '18 at 16:31
  • I imported HashSet and Set in the class containing my static method and the class with the executable code calling the method, but did not implement hashCode in either class. – Todd R Jan 28 '18 at 16:33
  • And tell us what your actual question/problem is. – JB Nizet Jan 28 '18 at 16:37
  • 3
    The whole point of a Set is to provide a fast contain() method. Iterating over all the elements and compare them with equals() defeats the whole purpose. Your method and retainAll do completely different things, so they're not comparable. Also, you should really learn the Java naming conventions, and use generics. We're not in 2005 anymore. – JB Nizet Jan 28 '18 at 16:39
  • I think that my confusion was either due to my failure to understand that retainAll actually returns a boolean that is true if the Set was changed, or by an error in a portion of my code that I didn't post here. I was able to get things working. – Todd R Jan 28 '18 at 19:39
  • My new executable code: String rowOne = "qwerty test String"; Set triosOne = MethodClass.get3CharSubstrings(rowOne, 3); String testKeySeq = "qwe"; System.out.println("Substrings of tested String: "+MethodClass.get3CharSubstrings(testKeySeq, 3)); boolean verdictTwo=MethodClass.contains3CharSubstring(testKeySeq, triosOne); System.out.println(testKeySeq+" keyboard sequential keys contained status: "+verdictTwo); Result: Substrings of tested String: [qwe] qwe keyboard sequential keys contained status: true – Todd R Jan 28 '18 at 19:44
  • The relevant method called is now coded this way: static boolean contains3CharSubstring(String tested, Set trios, int three) { int tlen = tested.length(); if (tlen < three) { return false; } if (trios.isEmpty()) { return false; } Set testTrios = get3CharSubstrings(tested, 3); testTrios.retainAll(trios); return !testTrios.isEmpty(); } – Todd R Jan 28 '18 at 19:45

3 Answers3

2

This statement from your question:

However, this method seems to test for equality using the == style check for whether they are references to the same memory object, rather than using the String's equals() method to check whether the contents are the same

is wrong. retainAll does use contains, which in turn uses equals.

I don't fully understand your use case, but I think you might find the Collections.disjoint method more useful than retainAll. From the docs:

Returns true if the two specified collections have no elements in common.

You could use it like this:

if (!Collections.disjoint(setOne, setTwo)) {
    // sets have at least one element in common
}

I'm proposing you use this method because retainAll modifies the set on which it's invoked on. Actually, it removes all the elements from this collection that are not contained in the argument collection. And from your code, it doesn't seem like you want this behavior.

fps
  • 33,623
  • 8
  • 55
  • 110
0

Actually retainsAll use contains that itself use equals, at least the standard version. Maybe you actually used an IdentityHashMap instead that would indeed use the memory reference for equality, but that would be because you asked for it.

public boolean  [More ...] retainAll(Collection<?> c) {
        boolean modified = false;
        Iterator<E> e = iterator();
        while (e.hasNext()) {
            if (!c.contains(e.next())) {
                e.remove();
                modified = true;
            }
        }
        return modified;
    }

     public boolean  [More ...] contains(Object o) {
         Iterator<E> e = iterator();
        if (o==null) {
            while (e.hasNext())
                if (e.next()==null)
                    return true;
        } else 
            while (e.hasNext()
                if (o.equals(e.next()))
                    return true;
        }
        return false;
    }

Next time, please consider using the debugger to double check (even code from the JDK) or google it (like HashSet.retainAll code source) you would find something like that: http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/HashSet.java

This is what I did to respond to your question.

Nicolas Bousquet
  • 3,990
  • 1
  • 16
  • 18
0

If you check OpenJDK9 source code, you can see that retainAll() uses AbstractCollection.contains(Object o):

public boolean retainAll(Collection<?> c) {
    Objects.requireNonNull(c);
    boolean modified = false;
    Iterator<E> it = iterator();
    while (it.hasNext()) {
        if (!c.contains(it.next())) {
            it.remove();
            modified = true;
        }
    }
    return modified;
}

Documentation of contains() says:

Returns true if this collection contains the specified element. More formally, returns true if and only if this collection contains at least one element e such that (o==null ? e==null : o.equals(e)).

Hence retainAll() is based on equals() check, which is what you want.

Volodymyr Kononenko
  • 478
  • 4
  • 7