Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » scout » Possible bug in class AbstractTable
Possible bug in class AbstractTable [message #1412177] Mon, 25 August 2014 13:32 Go to next message
Fredrik Möller is currently offline Fredrik MöllerFriend
Messages: 15
Registered: April 2013
Junior Member
The method here under is copied from the class org.eclipse.scout.rt.client.ui.basic.table.AbstractTable from JAR: org.eclipse.scout.rt.client_3.9.2.20140205-1709.jar

It think that it contains a bug.

If targetIndex is larger than sourceIndex then the row to move will be inserted at the incorrect position (+1)?

private void moveRowImpl(int sourceIndex, int targetIndex) {
    if (sourceIndex < 0) {
      sourceIndex = 0;
    }
    if (sourceIndex >= getRowCount()) {
      sourceIndex = getRowCount() - 1;
    }
    if (targetIndex < 0) {
      targetIndex = 0;
    }
    if (targetIndex >= getRowCount()) {
      targetIndex = getRowCount() - 1;
    }
    if (targetIndex != sourceIndex) {
      synchronized (m_cachedRowsLock) {
        m_cachedRows = null;
      }
      ITableRow row = m_rows.remove(sourceIndex);
      m_rows.add(targetIndex, row);
      // update row indexes
      int min = Math.min(sourceIndex, targetIndex);
      int max = Math.max(sourceIndex, targetIndex);
      ITableRow[] changedRows = new ITableRow[max - min + 1];
      for (int i = min; i <= max; i++) {
        changedRows[i - min] = getRow(i);
        ((InternalTableRow) changedRows[i - min]).setRowIndex(i);
      }
      fireRowOrderChanged();
      // rebuild selection
      selectRows(getSelectedRows(), false);
    }
  }


Kind regards
Fredrik Möller
Re: Possible bug in class AbstractTable [message #1412237 is a reply to message #1412177] Mon, 25 August 2014 16:29 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie BressonFriend
Messages: 1250
Registered: October 2011
Senior Member
Thanks for this information.

I have checked in the current version (Luna & Mars) of Eclipse Scout. With Bug 421181 Beat Schwarzentrub changed something in the method you are indicating. I doubt that it has something to do with your problem but it would be nice to know.

Other question:
The method you have mentioned is private API. Have you small use cases (maybe in form of a Unit Test [see TableTest for examples]) using public API that demonstrate when the error occurs.

Thank you in advance.

.
Re: Possible bug in class AbstractTable [message #1412418 is a reply to message #1412237] Tue, 26 August 2014 05:35 Go to previous messageGo to next message
Fredrik Möller is currently offline Fredrik MöllerFriend
Messages: 15
Registered: April 2013
Junior Member
Hello Jeremie

The problem could be triggered by using the public mehtod:

  @Override
  public void moveRow(int sourceIndex, int targetIndex) {
    moveRowImpl(sourceIndex, targetIndex);
  }


and using:

sourceIndex == 0
targetIndex == 1 (or any larger value)

The use case is that I want to move a table row from the original position of index 0 to index 1, one row below. Because of the remove method invoked of the List all the elements "below" will be switched one step upwards and then when the element is inserted that insert does not take accont of this intermediate change...

Regards
Fredrik Möller
Re: Possible bug in class AbstractTable [message #1412653 is a reply to message #1412418] Tue, 26 August 2014 18:19 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie BressonFriend
Messages: 1250
Registered: October 2011
Senior Member
Maybe I do not get it.

Here is my setup.
Table Field:
@Order(10.0)
public class MyTableField extends AbstractTableField<MyTableField.Table> {

  @Override
  protected String getConfiguredLabel() {
    return TEXTS.get("MyTable");
  }

  @Override
  protected int getConfiguredGridW() {
    return 2;
  }

  @Override
  protected int getConfiguredGridH() {
    return 5;
  }

  @Order(10.0)
  public class Table extends AbstractExtensibleTable {

    /**
     * @return the TextColumn
     */
    public TextColumn getTextColumn() {
      return getColumnSet().getColumnByClass(TextColumn.class);
    }

    @Order(10.0)
    public class TextColumn extends AbstractStringColumn {

      @Override
      protected String getConfiguredHeaderText() {
        return TEXTS.get("Text");
      }
    }
  }
}


Table init:
Table table = getMyTableField().getTable();

ITableRow r;
r = table.addRow(table.createRow());
table.getTextColumn().setValue(r, "Lorem");

r = table.addRow(table.createRow());
table.getTextColumn().setValue(r, "Ipsum");

r = table.addRow(table.createRow());
table.getTextColumn().setValue(r, "Dolor");


Table is:
* Lorem
* Ipsum
* Dolor

Then I call (in a push button):
Table table = getMyTableField().getTable();
table.moveRow(0, 1);


Table is:
* Ipsum
* Lorem
* Dolor

The rows are swapped as expected.

What did I miss?

.
Re: Possible bug in class AbstractTable [message #1412865 is a reply to message #1412653] Wed, 27 August 2014 08:54 Go to previous messageGo to next message
Fredrik Möller is currently offline Fredrik MöllerFriend
Messages: 15
Registered: April 2013
Junior Member
Hello Jeremie

Sorry, that is my misstake.
The public method moveRow does what it should, but the other public method moveRowBefore has a problem when the sourceIndex is smaller than the targetIndex.

Regards
Fredrik Möller

Re: Possible bug in class AbstractTable [message #1412915 is a reply to message #1412865] Wed, 27 August 2014 10:58 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie BressonFriend
Messages: 1250
Registered: October 2011
Senior Member
Hi,

Thanks for the pointer. I have submitted Bug 442687.

This bug is a good example on how we want to work:

- write JUnit tests (for what is already working) for AbstractTable#moveRow(..), AbstractTable# moveRowBefore(..) and AbstractTable#moveRowAfter(..) [Ensure no regression]
=> Bug 402298 can be used to improve the Unit Test coverage without changing productive code.

Then:
- write JUnit tests to illustrate what is not working (use cases mentioned in Bug 442687 and maybe more).
- and fix the bug in the same commit.

I think this is a bug that is accessible for an external contributor (I did not look at it in detail).

If someone is interested, please let us know I will do my best to guide you through the process (I will dedicate time to help people to contribute to eclipse scout).

.
Re: Possible bug in class AbstractTable [message #1449588 is a reply to message #1412915] Tue, 21 October 2014 15:19 Go to previous message
Christian Ulrich is currently offline Christian UlrichFriend
Messages: 3
Registered: July 2010
Junior Member
Hi Frederik,

I submitted JUnit tests as well as a bugfix to current develop branch (Scout 4.2.0). Please see Bug 442687 for verification.

Best regards
Christian Ulrich
Previous Topic:The whole view button menu (AbstractOutlineViewButton) is lost after some time
Next Topic:Setting the focus on the first row a a table
Goto Forum:
  


Current Time: Wed Dec 13 19:06:42 GMT 2017

Powered by FUDForum. Page generated in 0.02124 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software