Eclipse Community Forums: Nebula » [CompositeTable] Header and row control construction

Help

Home

Home » Eclipse Projects » Nebula » [CompositeTable] Header and row control construction

Show: Today's Messages :: Show Polls :: Message Navigator

[CompositeTable] Header and row control construction [message #29101]

Sun, 18 February 2007 21:09

Eclipse User

Originally posted by: h2.miegel.org

Thanks for a great control. We have been struggling with the Table control
and the CompositeTable control looks like it has the flexibility and
in-place editing support that we need in our open source accounting program.

The CompositeTable implementation uses reflection to create copies of the
header and row controls. This code causes problems for us. 1) We cannot
use inner classes as header and row implementations because CompositeTable
then does not find the constructor due to the implicit parameter to the
outer class instance. 2) We cannot create additional constructors to the
header and row objects that take additional parameters (well, we can create
the constructors but they wouldn't be used by CompositeTable). The net
result is that we have no way of getting data into our header object short
of a hack. The set of columns depends on options set in the account being
listed and we allow the user to further configure the columns (the selected
columns being persisted in the workbench view state memento), so we cannot
have a header implementation class for each possible set of columns (there
are millions of possible sets of columns).

CompositeTable provides a listener that fires an event after each time a new
header or row is constructed. Apart from the fact that this is a rather
bizzare way of constructing an object, it did not work for us. Constructing
the columns in this listener did not work (Unfortunately I did not
investigate why not).

Constructing the row objects using a factory pattern would solve this
problem. David says that this would break Visual Editor support. I don't
understand why. Visual Editor would still be able to add the template
instances of the header and row object as is currently done. All that would
change is the way the visible copies would be created and those copies are
created by CompositeTable, not Visual Editor.

I have attached a patch that does this. I do not see any problem with
taking things further and requiring all header and row objects be derived
from an abstract base class thus removing the need for reflection, or moving
to responsibility for row control creation to the content provider.
However, the attached patch puts control construction into a method in the
template control, thus minimizing problems that may arise because of issues
of which I am not aware.

There is a pull here between the requirement to support the creation of
static columns and layouts at coding time using Visual Editor and our
requirement to be able to dynamically select columns and row control layout
at runtime. I am sure that with a few minor changes the two will prove
easily reconcilable.

- Nigel Westbury

Attachment: patch.txt
(Size: 8.08KB, Downloaded 243 times)

Report message to a moderator

Re: [CompositeTable] Header and row control construction [message #29180 is a reply to message #29101]

Mon, 19 February 2007 08:27

Thomas Schindl

Messages: 6651
Registered: July 2009

Senior Member

Hi,

I'm not one of the CompositeTable committers but as far as I know
committers are not allowed to integrate patches sent to the mailing list
because then it's hard to track later under which license you released
your improvement. That's why the normal way is file a bug to bugzilla
and include your code as an attachment. This way you automatically
release the code under EPL and it can be commited to CVS. The bug is
then referenced in the header section of the sources and one can easily
find it later on.

Tom

Nigel Westbury schrieb:
> Thanks for a great control. We have been struggling with the Table control
> and the CompositeTable control looks like it has the flexibility and
> in-place editing support that we need in our open source accounting program.
>
> The CompositeTable implementation uses reflection to create copies of the
> header and row controls. This code causes problems for us. 1) We cannot
> use inner classes as header and row implementations because CompositeTable
> then does not find the constructor due to the implicit parameter to the
> outer class instance. 2) We cannot create additional constructors to the
> header and row objects that take additional parameters (well, we can create
> the constructors but they wouldn't be used by CompositeTable). The net
> result is that we have no way of getting data into our header object short
> of a hack. The set of columns depends on options set in the account being
> listed and we allow the user to further configure the columns (the selected
> columns being persisted in the workbench view state memento), so we cannot
> have a header implementation class for each possible set of columns (there
> are millions of possible sets of columns).
>
> CompositeTable provides a listener that fires an event after each time a new
> header or row is constructed. Apart from the fact that this is a rather
> bizzare way of constructing an object, it did not work for us. Constructing
> the columns in this listener did not work (Unfortunately I did not
> investigate why not).
>
> Constructing the row objects using a factory pattern would solve this
> problem. David says that this would break Visual Editor support. I don't
> understand why. Visual Editor would still be able to add the template
> instances of the header and row object as is currently done. All that would
> change is the way the visible copies would be created and those copies are
> created by CompositeTable, not Visual Editor.
>
> I have attached a patch that does this. I do not see any problem with
> taking things further and requiring all header and row objects be derived
> from an abstract base class thus removing the need for reflection, or moving
> to responsibility for row control creation to the content provider.
> However, the attached patch puts control construction into a method in the
> template control, thus minimizing problems that may arise because of issues
> of which I am not aware.
>
> There is a pull here between the requirement to support the creation of
> static columns and layouts at coding time using Visual Editor and our
> requirement to be able to dynamically select columns and row control layout
> at runtime. I am sure that with a few minor changes the two will prove
> easily reconcilable.
>
> - Nigel Westbury
>
>
>
> ### Eclipse Workspace Patch 1.0
> #P org.eclipse.swt.nebula.widgets
> Index: src/org/eclipse/swt/nebula/widgets/compositetable/CompositeT able.java
> ============================================================ =======
> RCS file: /cvsroot/technology/org.eclipse.swt.nebula/org.eclipse.swt.n ebula.widgets/src/org/eclipse/swt/nebula/widgets/compositeta ble/CompositeTable.java,v
> retrieving revision 1.3
> diff -u -r1.3 CompositeTable.java
> --- src/org/eclipse/swt/nebula/widgets/compositetable/CompositeT able.java 6 Feb 2007 03:46:42 -0000 1.3
> +++ src/org/eclipse/swt/nebula/widgets/compositetable/CompositeT able.java 18 Feb 2007 21:03:03 -0000
> @@ -13,6 +13,7 @@
> package org.eclipse.swt.nebula.widgets.compositetable;
>
> import java.lang.reflect.Constructor;
> +import java.lang.reflect.Method;
> import java.util.ArrayList;
> import java.util.Iterator;
> import java.util.LinkedList;
> @@ -140,10 +141,14 @@
> // Private fields here
> private Constructor headerConstructor = null;
>
> + private Method headerFactoryMethod = null;
> +
> private Control headerControl = null;
>
> private Constructor rowConstructor = null;
>
> + private Method rowFactoryMethod = null;
> +
> private Control rowControl = null;
>
> // TODO: on public API methods that reference contentPane, make sure it's
> @@ -337,27 +342,45 @@
> private void findPrototypeConstructors(Control[] finalChildren) {
> // Get a constructor for the header and/or the row prototype
> if (finalChildren.length == 1) {
> + rowControl = finalChildren[0];
> + } else {
> + rowControl = finalChildren[1];
> +
> + headerControl = finalChildren[0];
> try {
> - rowControl = (Composite) finalChildren[0];
> - rowConstructor = finalChildren[0].getClass().getConstructor(
> + headerConstructor = headerControl.getClass().getConstructor(
> new Class[] { Composite.class, Integer.TYPE });
> } catch (Exception e) {
> - throw new RuntimeException(
> - "Unable to get constructor object for header or row", e);
> + headerConstructor = null;
> }
> - } else {
> try {
> - headerConstructor = finalChildren[0].getClass().getConstructor(
> - new Class[] { Composite.class, Integer.TYPE });
> - headerControl = finalChildren[0];
> - rowConstructor = finalChildren[1].getClass().getConstructor(
> - new Class[] { Composite.class, Integer.TYPE });
> - rowControl = finalChildren[1];
> + headerFactoryMethod = headerControl.getClass().getMethod("createHeader",
> + new Class[] { Composite.class });
> } catch (Exception e) {
> + headerFactoryMethod = null;
> + }
> + if (headerConstructor == null && headerFactoryMethod == null) {
> throw new RuntimeException(
> - "Unable to get constructor object for header or row", e);
> + "Unable to get either constructor object or the createHeader method for header");
> }
> }
> +
> + try {
> + rowConstructor = rowControl.getClass().getConstructor(
> + new Class[] { Composite.class, Integer.TYPE });
> + } catch (Exception e) {
> + rowConstructor = null;
> + }
> + try {
> + rowFactoryMethod = rowControl.getClass().getMethod("createRow",
> + new Class[] { Composite.class });
> + } catch (Exception e) {
> + rowFactoryMethod = null;
> + }
> + if (rowConstructor == null && rowFactoryMethod == null) {
> + throw new RuntimeException(
> + "Unable to get either constructor object or the createRow method for row");
> + }
> }
>
> private void resizePrototypeObjects(Control[] finalChildren) {
> @@ -845,6 +868,27 @@
> }
>
> /**
> + * (non-API) Method getHeaderFactoryMethod. Returns the method in the header
> + * object used internally to construct the table's header, or null if there
> + * is none
> + *
> + * @return the header's constructor.
> + */
> + public Method getHeaderFactoryMethod() {
> + return headerFactoryMethod;
> + }
> +
> + /**
> + * (non-API) Method getRowFactoryMethod. Returns a method in the the object
> + * used internally to construct each row object
> + *
> + * @return the rows' constructor
> + */
> + public Method getRowFactoryMethod() {
> + return rowFactoryMethod;
> + }
> +
> + /**
> * (non-API) Method getHeaderControl. Returns the prototype header control.
> *
> * @return the prototype header control.
> Index: src/org/eclipse/swt/nebula/widgets/compositetable/InternalCo mpositeTable.java
> ============================================================ =======
> RCS file: /cvsroot/technology/org.eclipse.swt.nebula/org.eclipse.swt.n ebula.widgets/src/org/eclipse/swt/nebula/widgets/compositeta ble/InternalCompositeTable.java,v
> retrieving revision 1.14
> diff -u -r1.14 InternalCompositeTable.java
> --- src/org/eclipse/swt/nebula/widgets/compositetable/InternalCo mpositeTable.java 9 Feb 2007 03:36:47 -0000 1.14
> +++ src/org/eclipse/swt/nebula/widgets/compositetable/InternalCo mpositeTable.java 18 Feb 2007 21:03:04 -0000
> @@ -12,6 +12,7 @@
> package org.eclipse.swt.nebula.widgets.compositetable;
>
> import java.lang.reflect.Constructor;
> +import java.lang.reflect.Method;
> import java.util.Iterator;
> import java.util.LinkedList;
> import java.util.ListIterator;
> @@ -84,6 +85,10 @@
> // them
> private Constructor headerConstructor;
> private Constructor rowConstructor;
> +
> + private Method headerFactoryMethod;
> + private Method rowFactoryMethod;
> +
> private Control headerControl;
> private Control myHeader = null;
> private Control rowControl;
> @@ -112,6 +117,10 @@
>
> headerConstructor = parent.getHeaderConstructor();
> rowConstructor = parent.getRowConstructor();
> +
> + headerFactoryMethod = parent.getHeaderFactoryMethod();
> + rowFactoryMethod = parent.getRowFactoryMethod();
> +
> headerControl = parent.getHeaderControl();
> rowControl = parent.getRowControl();
>
> @@ -432,21 +441,38 @@
> * Construct a header or row object on demand. Logs an error and returns
> * null on failure.
> *
> + * At least one of <code>constructor</code> and <code>factoryMethod</code>
> + * must be non-null. If both are non-null then <code>factoryMethod</code>
> + * is used.
> + *
> * @param parent
> * The SWT parent.
> * @param constructor
> * The header or row object's constructor.
> + * @param factoryMethod
> + * A method in the header or row object that creates another
> + * copy of the header or row
> * @return The constructed control or null if none could be constructed.
> */
> private Control createInternalControl(Composite parent,
> - Constructor constructor) {
> + Control originalControl,
> + Constructor constructor, Method factoryMethod) {
> Control result = null;
> +
> try {
> - if (!constructor.isAccessible()) {
> - constructor.setAccessible(true);
> + if (factoryMethod != null) {
> + if (!factoryMethod.isAccessible()) {
> + factoryMethod.setAccessible(true);
> + }
> + result = (Control) factoryMethod.invoke(originalControl, new Object [] {
> + parent });
> + } else {
> + if (!constructor.isAccessible()) {
> + constructor.setAccessible(true);
> + }
> + result = (Control) constructor.newInstance(new Object[] { parent,
> + Integer.valueOf(originalControl.getStyle()) });
> }
> - result = (Control) constructor.newInstance(new Object[] { parent,
> - new Integer(SWT.NULL) });
> } catch (Exception e) {
> throw new IllegalArgumentException("Unable to construct control"); //$NON-NLS-1$
> }
> @@ -463,8 +489,11 @@
> * If the header control hasn't been created yet, create and show it.
> */
> private void showHeader() {
> - if (myHeader == null && headerConstructor != null) {
> - myHeader = createInternalControl(controlHolder, headerConstructor);
> + if (myHeader == null && (headerConstructor != null || headerFactoryMethod != null)) {
> + myHeader = createInternalControl(controlHolder,
> + headerControl,
> + headerConstructor,
> + headerFactoryMethod);
> fireHeaderConstructionEvent(myHeader);
> if (myHeader instanceof Composite) {
> Composite headerComp = (Composite) myHeader;
> @@ -771,7 +800,9 @@
> return recycledRow;
> }
> Control newControl = createInternalControl(controlHolder,
> - rowConstructor);
> + rowControl,
> + rowConstructor,
> + rowFactoryMethod);
> if (menu != null) {
> newControl.setMenu(menu);
> }

Report message to a moderator