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

IntMap.clear() does not work as expected #144

Closed
ghost opened this issue Nov 11, 2013 · 2 comments
Closed

IntMap.clear() does not work as expected #144

ghost opened this issue Nov 11, 2013 · 2 comments

Comments

@ghost
Copy link

ghost commented Nov 11, 2013

From romixlev on October 22, 2013 21:53:30

IntMap.clear() does not make a map empty. I run into this bug while experimenting with the DefaultClassResolver class who uses this method.

Here is a unit test to reproduce a problem:

package com.esotericsoftware.kryo;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.sql.Date;
import java.util.ArrayList;
import java.util.IdentityHashMap;
import java.util.List;

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.io.Input;
import com.esotericsoftware.kryo.io.Output;
import com.esotericsoftware.kryo.util.IntMap;
import com.esotericsoftware.minlog.Log;

public class IntMapTest extends KryoTestCase {

public void testCase () throws Exception {
    IntMap<Object> map = new IntMap<Object>();
    map.put(9, new Object());
    map.clear();
    assertNull("Key 9 should not be present", map.get(9));
    System.out.println(map);
}

}

My current proposal for a fix would be this:
public void clear () {
int[] keyTable = this.keyTable;
V[] valueTable = this.valueTable;

    for (int i = capacity + stashSize; i-- > 0;) {
        keyTable[i] = EMPTY;
        valueTable[i] = null;
    }

// for (int i = size - 1; i >= 0; i--) {
// keyTable[i] = EMPTY;
// valueTable[i] = null;
// }
// for (int i = capacity, n = i + stashSize - 1; i < n; i++) {
// keyTable[i] = EMPTY;
// valueTable[i] = null;
// }
size = 0;
stashSize = 0;
zeroValue = null;
hasZeroValue = false;
}

But I'm wondering what is the current logic in the code which I commented out? May be there is a more efficient way to clear a map than what I suggested above?

@nate: Do you remember why it is written this way currently?

Original issue: http://code.google.com/p/kryo/issues/detail?id=144

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on October 22, 2013 13:04:53

Aye, it is currently wrong. Should be like this:
https:/libgdx/libgdx/blob/master/gdx/src/com/badlogic/gdx/utils/IntMap.java#L414

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From romixlev on October 22, 2013 13:10:38

(No comment was entered for this change.)

Status: Fixed

@ghost ghost closed this as completed Nov 11, 2013
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

0 participants