_E_HashSet.template retainAll() hidden side effect

Issue #53 new
Roman Chernyatchik
created an issue

retainAll() methods changes array passed as argument, actually it sorts the array. Such behavior isn't mentioned in javadoc and can lead to hard-to-debug bugs.

/** {@inheritDoc} */
    public boolean retainAll( #e#[] array ) {
        boolean changed = false;
        Arrays.sort( array );
        #e#[] set = _set;
        byte[] states = _states;

        // ...     
        return changed;
    }

Minimal test case:

public class EHashSetTestCase extends TestCase {
  public void testRetainAllSideEffect() throws Exception {
    final int[] data = {5, 4, 3};

    final TIntHashSet seen = new TIntHashSet(new int[]{1, 2, 3});

    // Precondition
    assertEquals("[5, 4, 3]", Arrays.toString(data));

    seen.retainAll(data);

    // Test
    assertEquals("[5, 4, 3]", Arrays.toString(data));
  }
}

Test fails: junit.framework.ComparisonFailure: Expected :[5, 4, 3] Actual :[3, 4, 5]

Comments (4)

  1. jimdavies

    Seems pretty clear cut. I think that modifying the array passed in to the retainAll method is surprising behaviour, so it seems fair to say that it shouldn't happen that way.

    I'll check why the array needs to be modified. If there is a simple mitigation, I'll code that. Otherwise, I'll clone the array, and work with the clone.

  2. Log in to comment