There is somes bug in the “Table” class.
The removeRow method is currently like this
[java]
public void removeRow(int index, boolean pack) {
selectedCells.remove(index);
selectedRows.remove(index); //<- the bug is here
this.getVScrollBar().hide();
if (!rows.isEmpty()) {
if (index >= 0 && index < rows.size()) {
rows.remove(index);
pack(); //bug also here ?
}
}
}
[/java]
The first is a real bug. When you do “selectedCells.remove(index);” it’s perfectly fine as selectedCells is a Map and a map has only a method to remove a key. So the method with the Object parameter is selected.
However, when you do the same call with selectedRows, you do it on a List. A List has 2 “remove” methods:
remove(int index)
remove(Object o)
so, when you do “selectedRows.remove(index);” you DON’T remove the integer “index” from the list, you try to remove the nth (with n = index) element of the list, and you get an ArrayIndexOutOfBoundsException.
The fix is:
[java]
public void removeRow(int index, boolean pack) {
selectedCells.remove(index);
selectedRows.remove(Integer.valueOf(index)); // transform the int into Integer, so the method with an Object as parameter is used.
this.getVScrollBar().hide();
if (!rows.isEmpty()) {
if (index >= 0 && index < rows.size()) {
rows.remove(index);
pack();
}
}
}
[/java]
I also think that the pack method should be enclosed in an “if (pack) { }” (you have it when you add a row).
For the second thing, it’s not really a bug but it’s something a bit confusing: the insertrow method is currently like this:
[java]
public void insertRow(int index, TableRow row, boolean pack) {
if (!rows.isEmpty()) {
if (index >= 0 && index < rows.size()) {
this.getVScrollBar().hide();
this.rows.add(index, row);
if (pack) {
pack();
}
}
}
}
[/java]
So, you can’t insert a row in an empty table or at the end of the list. Maybe that it’s a design choice, but it is just unintuitive, especially when the List (the standard collection) chose the opposite.
Maybe that something like this would be better:
[java]
@Override
public void insertRow(int index, TableRow row, boolean pack)
{
int s = rows.size();
if (index >= 0 && index < s)
{
this.getVScrollBar().hide();
this.rows.add(index, row);
if (pack)
{
pack();
}
}
else if (index == s)
{
addRow(row, pack);
}
else
{
//warn, maybe with a logger
}
}
[/java]
Also, for people that still has a “buggy” version and want a temporary fix, i wrote this class that extends Table and changes methods as described above (it doesn’t contain the “if” thing ,as i don’t know if it has been omitted on purpose):
[java]
/**
- Temporary fix for the Table class
-
@author Bubuche
*/
public abstract class ETable extends Table
{
private static final Logger LOG = Logger.getLogger(ETable.class.getName());
protected List<TableRow> rows = new ArrayList();
protected List<Integer> selectedRows = new ArrayList();
protected Map<Integer, List<Integer>> selectedCells;
public ETable(ElementManager screen, Vector2f position)
{
super(screen, position);
init();
}
public ETable(ElementManager screen, Vector2f position, Vector2f dimensions)
{
super(screen, position, dimensions);
init();
}
public ETable(ElementManager screen, Vector2f position, Vector2f dimensions, Vector4f resizeBorders, String defaultImg)
{
super(screen, position, dimensions, resizeBorders, defaultImg);
init();
}
public ETable(ElementManager screen, String UID, Vector2f position)
{
super(screen, UID, position);
init();
}
public ETable(ElementManager screen, String UID, Vector2f position, Vector2f dimensions)
{
super(screen, UID, position, dimensions);
init();
}
public ETable(ElementManager screen, String UID, Vector2f position, Vector2f dimensions, Vector4f resizeBorders, String defaultImg)
{
super(screen, UID, position, dimensions, resizeBorders, defaultImg);
init();
}
private void init()
{
try
{
Field field;
field = Table.class.getDeclaredField("selectedRows");
field.setAccessible(true);
this.selectedRows = (List<Integer>) field.get(this);
field = Table.class.getDeclaredField("selectedCells");
field.setAccessible(true);
this.selectedCells = (Map<Integer, List<Integer>>) field.get(this);
field = Table.class.getDeclaredField("rows");
field.setAccessible(true);
this.rows = (List<TableRow>) field.get(this);
}
catch (NoSuchFieldException | SecurityException | IllegalArgumentException |
IllegalAccessException ex)
{
Logger.getLogger(ETable.class.getName()).log(Level.SEVERE, null, ex);
}
}
@Override
public void removeRow(int index, boolean pack)
{
Integer index_as_object = Integer.valueOf(index);
selectedCells.remove(index);
selectedRows.remove(index_as_object);
this.getVScrollBar().hide();
if (!rows.isEmpty())
{
if (index >= 0 && index < rows.size())
{
rows.remove(index);
pack(); //should have "if ( pack )", shouldn't it ?
}
}
}
@Override
public void insertRow(int index, TableRow row, boolean pack)
{
int s = rows.size();
if (index >= 0 && index < s)
{
this.getVScrollBar().hide();
this.rows.add(index, row);
if (pack)
{
pack();
}
}
else if (index == s)
{
addRow(row, pack);
}
else
{
LOG.log(Level.WARNING, “Invalid index: {0} (max index allowed: {1}”, new Object[]{index, s});
}
}
}
[/java]
And … " @t0neg0d see this message" (like a divin invocation).
