Hierarchical Glazed Lists

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Hierarchical Glazed Lists

Bruce Alspaugh-2
I'm working on an enhancement to Glazed Lists that would bind the lists
to JFace viewers instead of the underlying SWT Table.  JFace also
supports TreeViewers and TableTreeViewers.  I'm trying to figure out if
it's practical to display Glazed Lists in these hierarchical widgets.  
Can you do that in Swing?

I'm trying to figure the best way to model this.  Do you support having
hierarchical lists where each list element is another list instead of a
JavaBean with a list property?  How would you sort and filter a list of
lists?

Bruce

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

James Lemieux
Bruce,

   You are right on the bleeding edge. Jesse and I have fixed our attention on treetable support "Glazed-Lists style" for the last month or so. Our work is just starting to bare fruit. The implementation is fairly virgin, but we'd LOVE it if you'd be the first to kick the tires. You'd also have to do some SWT work first, since only Swing bindings have been started so far, but at least the approach has been largely proved out.

   Ok, so you may have realized that a new extension has existed for a month or so called "treetable." The idea we're following here is that a treetable is really just a standard table with a single column that displays and edits the tree's hierarchy. All of the rest of the table behaviour is basically what you want in a treetable.

   You may be thinking "Big deal, most treetable implementations behave that way!" But, the big catch is that Glazed Lists allows you, in a very declarative way, to define the logic that calculates a "path to the tree root" for each element in a source list, to produce a TreeList. After creating the function that effectively transforms a raw EventList into a TreeList, you can start asking that TreeList tree-like questions about its elements. (can the TreeList element at index 7 be expanded? is the TreeList element at index 7 expanded? what is the depth of the TreeList element at index 7?)

i.e. you can do this:

// 1. make a list of cars
EventList cars = ...

// 2. transform the list of cars into a *tree* of cars using a TreeFormat object
TreeList carsByMakeByPrice = new TreeList(cars, new CarsByMakeByPriceTreeFormat());

// 3. make a normal TableModel for the TreeList
TableModel carTableModel = new EventTableModel(carsByMakeByPrice, new CarTableFormat());

JTable carTable = new JTable(carTableModel);

// 4. make the first column of the table a "hierarchical column" which displays the hierarchy
TreeTableSupport.install(carTable, carsByMakeByPrice, 0);



So, let's look at the data at each step of the way in the above code:

1. (raw list of cars)
new Car("ES300", 33000, "Lexus");
new Car("LS400", 58000, "Lexus");
new Car("Outback", 28000, "Subaru");

2. (TreeList contains both the Car objects from the source list as well as "synthetic" Car objects added by TreeList.TreeFormat object to describe hierarchy)
new Car("Lexus", 0, "");               // synthetic
new Car("30000-35000", 0, "");         // synthetic
new Car("ES300", 33000, "Lexus");      // source element 0
new Car("55000-60000", 0, "");         // synthetic
new Car("LS400", 58000, "Lexus");      // source element 1
new Car("Subaru", 0, "");              // synthetic
new Car("25000-30000", 0, "");         // synthetic
new Car("Outback", 28000, "Subaru");   // source element 2

3. Raw tabular data from the TreeList of cars as defined by the CarTableFormat
"Lexus"                0       ""
"30000-35000"          0       ""
"ES300"            33000       "Lexus"
"55000-60000"          0       ""
"LS400"            58000       "Lexus"
"Subaru"               0       ""
"25000-30000"          0       ""
"Outback"          28000       "Subaru"

4. The treetable data where the indenting and the collapse/expand information is provided by the TreeList to special renderers and editors installed on the "hierarchical TableColumn" by TreeTableSupport
- "Lexus"              0       ""
  - "30000-35000"      0       ""
      "ES300"      33000       "Lexus"
  - "55000-60000"      0       ""
      "LS400"      58000       "Lexus"
- "Subaru"             0       ""
  - "25000-30000"      0       ""
      "Outback"    28000       "Subaru"


You can see all of this at work in a new demo application: AmazonBrowser (available in the latest jars... though I'd recommend running from your IDE if you have the latest CVS source code). AmazonBrowser lets you query for Amazon.com products via their webservice. It displays the products in a rudimentary treetable.

If you are game, now is a GREAT time to consider jumping into Glazed Lists. The reason is that TreeList can be largely treated like a black box (in which Jesse toils and slaves for us), and the UI code is actually largely trivial moderate complexity (or at least the Swing code is). The SWT equivalent of swing's TreeTableSupport needs to be written.

Our approach to TreeTables is NOT the be-all-end-all TreeTable. I admit that there are realworld use cases that to which we cannot cater... ones in which the algorithm for turning a list of objects into a tree of objects isn't so "pretty" ( e.g. the same list element should be displayed MULTIPLE times in your tree). But, for those that want to INFER a relatively uniform hierarchy from a list of objects, this approach is quite powerful and can save a lot of time and effort.

James

On 8/31/06, Bruce Alspaugh <[hidden email]> wrote:
I'm working on an enhancement to Glazed Lists that would bind the lists
to JFace viewers instead of the underlying SWT Table.  JFace also
supports TreeViewers and TableTreeViewers.  I'm trying to figure out if
it's practical to display Glazed Lists in these hierarchical widgets.
Can you do that in Swing?

I'm trying to figure the best way to model this.  Do you support having
hierarchical lists where each list element is another list instead of a
JavaBean with a list property?  How would you sort and filter a list of
lists?

Bruce

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

Bruce Alspaugh-2
Sounds just like what I need.  I'll take a look at TreeList and see if I can bind it into a JFace TableTreeViewer.  Do you have any screencasts for the Swing version?

By the way, I'm getting along well binding EventLists to JFace TableViewers.  I solved the image loading problem for the sort icons the other day, and I was experimenting with in-cell editing today.  I keep running into bugs and ongoing construction work on the JFace API, so I guess I'm on the bleeding edge there too.  My goal is to produce a family of decorators that bind EventLists to various JFace viewers.  Hopefully, the TreeList will make the TableTreeViewer binding a lot easier.

Bruce

PS: 
On 8/31/06, James Lemieux <[hidden email]> wrote:
Bruce,

   You are right on the bleeding edge. Jesse and I have fixed our attention on treetable support "Glazed-Lists style" for the last month or so. Our work is just starting to bare fruit. The implementation is fairly virgin, but we'd LOVE it if you'd be the first to kick the tires. You'd also have to do some SWT work first, since only Swing bindings have been started so far, but at least the approach has been largely proved out.

   Ok, so you may have realized that a new extension has existed for a month or so called "treetable." The idea we're following here is that a treetable is really just a standard table with a single column that displays and edits the tree's hierarchy. All of the rest of the table behaviour is basically what you want in a treetable.

   You may be thinking "Big deal, most treetable implementations behave that way!" But, the big catch is that Glazed Lists allows you, in a very declarative way, to define the logic that calculates a "path to the tree root" for each element in a source list, to produce a TreeList. After creating the function that effectively transforms a raw EventList into a TreeList, you can start asking that TreeList tree-like questions about its elements. (can the TreeList element at index 7 be expanded? is the TreeList element at index 7 expanded? what is the depth of the TreeList element at index 7?)

i.e. you can do this:

// 1. make a list of cars
EventList cars = ...

// 2. transform the list of cars into a *tree* of cars using a TreeFormat object
TreeList carsByMakeByPrice = new TreeList(cars, new CarsByMakeByPriceTreeFormat());

// 3. make a normal TableModel for the TreeList
TableModel carTableModel = new EventTableModel(carsByMakeByPrice, new CarTableFormat());

JTable carTable = new JTable(carTableModel);

// 4. make the first column of the table a "hierarchical column" which displays the hierarchy
TreeTableSupport.install(carTable, carsByMakeByPrice, 0);



So, let's look at the data at each step of the way in the above code:

1. (raw list of cars)
new Car("ES300", 33000, "Lexus");
new Car("LS400", 58000, "Lexus");
new Car("Outback", 28000, "Subaru");

2. (TreeList contains both the Car objects from the source list as well as "synthetic" Car objects added by TreeList.TreeFormat object to describe hierarchy)
new Car("Lexus", 0, "");               // synthetic
new Car("30000-35000", 0, "");         // synthetic
new Car("ES300", 33000, "Lexus");      // source element 0
new Car("55000-60000", 0, "");         // synthetic
new Car("LS400", 58000, "Lexus");      // source element 1
new Car("Subaru", 0, "");              // synthetic
new Car("25000-30000", 0, "");         // synthetic
new Car("Outback", 28000, "Subaru");   // source element 2

3. Raw tabular data from the TreeList of cars as defined by the CarTableFormat
"Lexus"                0       ""
"30000-35000"          0       ""
"ES300"            33000       "Lexus"
"55000-60000"          0       ""
"LS400"            58000       "Lexus"
"Subaru"               0       ""
"25000-30000"          0       ""
"Outback"          28000       "Subaru"

4. The treetable data where the indenting and the collapse/expand information is provided by the TreeList to special renderers and editors installed on the "hierarchical TableColumn" by TreeTableSupport
- "Lexus"              0       ""
  - "30000-35000"      0       ""
      "ES300"      33000       "Lexus"
  - "55000-60000"      0       ""
      "LS400"      58000       "Lexus"
- "Subaru"             0       ""
  - "25000-30000"      0       ""
      "Outback"    28000       "Subaru"


You can see all of this at work in a new demo application: AmazonBrowser (available in the latest jars... though I'd recommend running from your IDE if you have the latest CVS source code). AmazonBrowser lets you query for <a href="http://Amazon.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">Amazon.com products via their webservice. It displays the products in a rudimentary treetable.

If you are game, now is a GREAT time to consider jumping into Glazed Lists. The reason is that TreeList can be largely treated like a black box (in which Jesse toils and slaves for us), and the UI code is actually largely trivial moderate complexity (or at least the Swing code is). The SWT equivalent of swing's TreeTableSupport needs to be written.

Our approach to TreeTables is NOT the be-all-end-all TreeTable. I admit that there are realworld use cases that to which we cannot cater... ones in which the algorithm for turning a list of objects into a tree of objects isn't so "pretty" ( e.g. the same list element should be displayed MULTIPLE times in your tree). But, for those that want to INFER a relatively uniform hierarchy from a list of objects, this approach is quite powerful and can save a lot of time and effort.

James


On 8/31/06, Bruce Alspaugh <[hidden email]> wrote:
I'm working on an enhancement to Glazed Lists that would bind the lists
to JFace viewers instead of the underlying SWT Table.  JFace also
supports TreeViewers and TableTreeViewers.  I'm trying to figure out if
it's practical to display Glazed Lists in these hierarchical widgets.
Can you do that in Swing?

I'm trying to figure the best way to model this.  Do you support having
hierarchical lists where each list element is another list instead of a
JavaBean with a list property?  How would you sort and filter a list of
lists?

Bruce

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]



Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

Danilo Tuler-2
Hi Bruce and everybody,

> By the way, I'm getting along well binding EventLists to JFace TableViewers.

I read other emails regarding your SWT effort.
A couple of weeks ago I also wrote a TableViewer support implementation.
It looks similar to your solution...
I also implemented support for ILazyContentProvider, which provides
elements on demand. It's working fine.

And I started to develop a RCP IssuesBrower.
This could be a nice testbed for the SWT support.

You can try to join efforts.
Is your code available in any CVS or SVN repository?

Finally I thank you guys for the great work on GL.
It's one of the finest piece of software I've used so far.
Let's give some attention to the SWT support, please! :-)

Thanks,
Danilo

ps: I'm also very interested in the TreeViewer support, but I think we
must focus on the table first.

ps2: I should have answered the other thread, but I just signed the
dev list, and didn't know how to reply that. sorry.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

James Lemieux
In reply to this post by Bruce Alspaugh-2
No screencasts yet Bruce. I suppose I could create one if it would help you understand what is there so far. It's a little premature for one, as normally we'll have the APIs much closer to being "frozen" before we generate some public documentation, but I think this is a fine time to break with tradition.

Glad to hear the JFace binding is going so well. If you feel like submitting it back to Glazed Lists, we'll gladly take a look at it and find a home for it.

James

On 8/31/06, Bruce Alspaugh <[hidden email]> wrote:
Sounds just like what I need.  I'll take a look at TreeList and see if I can bind it into a JFace TableTreeViewer.  Do you have any screencasts for the Swing version?

By the way, I'm getting along well binding EventLists to JFace TableViewers.  I solved the image loading problem for the sort icons the other day, and I was experimenting with in-cell editing today.  I keep running into bugs and ongoing construction work on the JFace API, so I guess I'm on the bleeding edge there too.  My goal is to produce a family of decorators that bind EventLists to various JFace viewers.  Hopefully, the TreeList will make the TableTreeViewer binding a lot easier.

Bruce

PS: 

On 8/31/06, James Lemieux <[hidden email]> wrote:
Bruce,

   You are right on the bleeding edge. Jesse and I have fixed our attention on treetable support "Glazed-Lists style" for the last month or so. Our work is just starting to bare fruit. The implementation is fairly virgin, but we'd LOVE it if you'd be the first to kick the tires. You'd also have to do some SWT work first, since only Swing bindings have been started so far, but at least the approach has been largely proved out.

   Ok, so you may have realized that a new extension has existed for a month or so called "treetable." The idea we're following here is that a treetable is really just a standard table with a single column that displays and edits the tree's hierarchy. All of the rest of the table behaviour is basically what you want in a treetable.

   You may be thinking "Big deal, most treetable implementations behave that way!" But, the big catch is that Glazed Lists allows you, in a very declarative way, to define the logic that calculates a "path to the tree root" for each element in a source list, to produce a TreeList. After creating the function that effectively transforms a raw EventList into a TreeList, you can start asking that TreeList tree-like questions about its elements. (can the TreeList element at index 7 be expanded? is the TreeList element at index 7 expanded? what is the depth of the TreeList element at index 7?)

i.e. you can do this:

// 1. make a list of cars
EventList cars = ...

// 2. transform the list of cars into a *tree* of cars using a TreeFormat object
TreeList carsByMakeByPrice = new TreeList(cars, new CarsByMakeByPriceTreeFormat());

// 3. make a normal TableModel for the TreeList
TableModel carTableModel = new EventTableModel(carsByMakeByPrice, new CarTableFormat());

JTable carTable = new JTable(carTableModel);

// 4. make the first column of the table a "hierarchical column" which displays the hierarchy
TreeTableSupport.install(carTable, carsByMakeByPrice, 0);



So, let's look at the data at each step of the way in the above code:

1. (raw list of cars)
new Car("ES300", 33000, "Lexus");
new Car("LS400", 58000, "Lexus");
new Car("Outback", 28000, "Subaru");

2. (TreeList contains both the Car objects from the source list as well as "synthetic" Car objects added by TreeList.TreeFormat object to describe hierarchy)
new Car("Lexus", 0, "");               // synthetic
new Car("30000-35000", 0, "");         // synthetic
new Car("ES300", 33000, "Lexus");      // source element 0
new Car("55000-60000", 0, "");         // synthetic
new Car("LS400", 58000, "Lexus");      // source element 1
new Car("Subaru", 0, "");              // synthetic
new Car("25000-30000", 0, "");         // synthetic
new Car("Outback", 28000, "Subaru");   // source element 2

3. Raw tabular data from the TreeList of cars as defined by the CarTableFormat
"Lexus"                0       ""
"30000-35000"          0       ""
"ES300"            33000       "Lexus"
"55000-60000"          0       ""
"LS400"            58000       "Lexus"
"Subaru"               0       ""
"25000-30000"          0       ""
"Outback"          28000       "Subaru"

4. The treetable data where the indenting and the collapse/expand information is provided by the TreeList to special renderers and editors installed on the "hierarchical TableColumn" by TreeTableSupport
- "Lexus"              0       ""
  - "30000-35000"      0       ""
      "ES300"      33000       "Lexus"
  - "55000-60000"      0       ""
      "LS400"      58000       "Lexus"
- "Subaru"             0       ""
  - "25000-30000"      0       ""
      "Outback"    28000       "Subaru"


You can see all of this at work in a new demo application: AmazonBrowser (available in the latest jars... though I'd recommend running from your IDE if you have the latest CVS source code). AmazonBrowser lets you query for <a href="http://Amazon.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">Amazon.com products via their webservice. It displays the products in a rudimentary treetable.

If you are game, now is a GREAT time to consider jumping into Glazed Lists. The reason is that TreeList can be largely treated like a black box (in which Jesse toils and slaves for us), and the UI code is actually largely trivial moderate complexity (or at least the Swing code is). The SWT equivalent of swing's TreeTableSupport needs to be written.

Our approach to TreeTables is NOT the be-all-end-all TreeTable. I admit that there are realworld use cases that to which we cannot cater... ones in which the algorithm for turning a list of objects into a tree of objects isn't so "pretty" ( e.g. the same list element should be displayed MULTIPLE times in your tree). But, for those that want to INFER a relatively uniform hierarchy from a list of objects, this approach is quite powerful and can save a lot of time and effort.

James


On 8/31/06, Bruce Alspaugh <[hidden email]> wrote:
I'm working on an enhancement to Glazed Lists that would bind the lists
to JFace viewers instead of the underlying SWT Table.  JFace also
supports TreeViewers and TableTreeViewers.  I'm trying to figure out if
it's practical to display Glazed Lists in these hierarchical widgets.
Can you do that in Swing?

I'm trying to figure the best way to model this.  Do you support having
hierarchical lists where each list element is another list instead of a
JavaBean with a list property?  How would you sort and filter a list of
lists?

Bruce

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]




Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

Bruce Alspaugh-2
In reply to this post by Danilo Tuler-2
I've attached what I have written so far.  It's very preliminary and
needs more testing.

There are a lot of bug fixes, backports, and API construction work going
on in JFace right now.  Hopefully, by Eclipse 3.3 JFace will be much
more stable.  I've stayed away from using virtual tables because of
known bugs like:  https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755

I'm not sure the best way to submit the JFace extension that builds on
the SWT extension.  I need two more packages:  
ca.odell.glazedlists.jface and ca.odell.glazedlists.impl.jface.  The
second one would be tagged as "x-internal:=true" in the export package
list.  I also need to add one more dependency to the manifest.mf so it
would read:

"Require-Bundle: org.eclipse.swt, org.eclipse.jface,
org.eclipse.core.runtime"

We need to support a plain ListViewer and the various checkbox lists
also.  I've started to work on that too.

Bruce

Danilo Tuler wrote:

> Hi Bruce and everybody,
>
>> By the way, I'm getting along well binding EventLists to JFace
>> TableViewers.
>
> I read other emails regarding your SWT effort.
> A couple of weeks ago I also wrote a TableViewer support implementation.
> It looks similar to your solution...
> I also implemented support for ILazyContentProvider, which provides
> elements on demand. It's working fine.
>
> And I started to develop a RCP IssuesBrower.
> This could be a nice testbed for the SWT support.
>
> You can try to join efforts.
> Is your code available in any CVS or SVN repository?
>
> Finally I thank you guys for the great work on GL.
> It's one of the finest piece of software I've used so far.
> Let's give some attention to the SWT support, please! :-)
>
> Thanks,
> Danilo
>
> ps: I'm also very interested in the TreeViewer support, but I think we
> must focus on the table first.
>
> ps2: I should have answered the other thread, but I just signed the
> dev list, and didn't know how to reply that. sorry.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

JFace Extension v20060901.zip (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

James Lemieux
Bruce,

   A few comments based on the source code you sent. They're meant to be constructive criticism about the code.


1. You have notes to this effect in your EventContentProvider:
                         /**
                         * Note: If I knew the index where where the insertion or
                         * deletion is taking place, I could optimise the refresh
                         * rather than redrawing the entire table.
                         */
                            case ListEvent.INSERT:
                            case ListEvent.DELETE:
                                tableViewer.refresh();
                                break;
                            case ListEvent.UPDATE :
                        /**
                         * Note:  If I knew the property being changed, I could
                         * optimize the update rather than redraw the entire row.
                         */
                                tableViewer.update(swtSource.get(aListEvent.getIndex()), null);
                                break;

and I wanted to let you know:

a) in the case of an inserts and deletes you *do* have the index of the insertion and deletion (it's available via aListEvent.getIndex(), just like you're using for updates)

b) Glazed Lists may not ever be able to furnish you with the property change so you can redraw just a cell rather than the entire row. In Swing, we also live with that limitation, though in practice the performance benefits are generally quite negligible for about 99% of all tables.

2. This is an important fix: EventContentProvider.listChanged(...) should *NOT* acquire the read lock for the swtSource. We've removed the acquisition of that lock from our swing EventTableModel recently because it can cause deadlock and it is unnecessary.

Specifically, it caused deadlock in 1.7.0 because we changed the EventSelectionModel to correctly start acquiring the writeLock when changes were made to it. That change created a situation where the Swing EDT would deadlock *itself* by first acquiring the readLock() in the EventTableModel and then later try to acquire the writeLock() from the same ReadWriteLock pair.

The reason that the lock is not needed is that we're reacting to a ListEvent there, implying that some Thread somewhere has made a change to the list, which in turn implies that Thread should be holding the writeLock() *already* if concurrent access is a problem for them. And if concurrent access isn't a problem then the readLock() *still* is unnecessary. Either way, the acquisition of the readLock in the listChanged() method is superfluous or plain wrong.

3. You're using an antipattern in the TableViewerManager constructor that has bitten me in the ass in the past. Specifically, you're using local factory methods to produce reasonable implementations of some JFace interfaces, like so:

tableViewer.setContentProvider(getContentProvider());
tableViewer.setLabelProvider(getLabelProvider());

Right now, it seems like a great idea because you've factored out two methods that could be overridden by subclasses, should they need to produce a "custom" content provider or label provider. But, there is a nasty problem with calling those methods from the constructor of the super class. The implied assumption in getContentProvider() and getLabelProvider() is that they don't require *any* member variable state (object state) in order to function. To put it more concretely, imagine I write something like this (even if it is a contrived bogus example):

public class MyCustomTableViewerManager extends TableViewerManager {

    private Object[] contentProviderElements;

    public MyCustomTableViewerManager(TableViewer aTableViewer, EventList aSourceList, TableFormat aTableFormat, Object[] contentProviderElements) {

       // this is going to call our definition of getContentProvider()
       super(aTableViewer, aSourceList, aTableFormat);

       this.contentProviderElements = contentProviderElements;
    }

    // I want a custom content provider, so I'm overriding
    public IStructuredContentProvider getContentProvider() {

        // when this line executes, the implied reference to the enclosing class is null!
        // i.e. MyCustomTableViewerManager.this == null
        return new IStructuredContentProvider() {
            public Object[] getElements(Object aInputElements) {
                // I want to return the same Object[] each time, no matter what
                return contentProviderElements;
            }

            public void inputChanged(Viewer aViewer, Object aOldInput, Object aNewInput) {
                // this should never happen - no-op implementation
            }
        }
    }
}

One of the best ways to combat the antipattern is simply to ask for the objects you need in the constructor, rather than using "local factory methods". It means callers have to do a bit more work, but it is work "on their own terms", rather than work to be done "at the time and location determined by the superclass." Overloading the constructor to include at least one "uber long and very explicit list of parameters" is probably something to consider, with the idea that about 95% of all API users will be able to get by with one of the smaller constructors anyway.

4. Regarding EventLabelProvider, if you think there are valid cases to extend that class, by all means expose it as a public class. If, however, you suspect there aren't many extension points, and you'd rather be conservative at first, we can throw it behind a factory method on GlazedListsSWT, and move EventLabelProvider into the impl package. (without knowing anything about expected usage, I'd probably start with the latter)

James

On 9/1/06, Bruce Alspaugh <[hidden email]> wrote:
I've attached what I have written so far.  It's very preliminary and
needs more testing.

There are a lot of bug fixes, backports, and API construction work going
on in JFace right now.  Hopefully, by Eclipse 3.3 JFace will be much
more stable.  I've stayed away from using virtual tables because of
known bugs like:  <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755

I'm not sure the best way to submit the JFace extension that builds on
the SWT extension.  I need two more packages:
ca.odell.glazedlists.jface and ca.odell.glazedlists.impl.jface.  The
second one would be tagged as "x-internal:=true" in the export package
list.  I also need to add one more dependency to the manifest.mf so it
would read:

"Require-Bundle: org.eclipse.swt, org.eclipse.jface,
org.eclipse.core.runtime"

We need to support a plain ListViewer and the various checkbox lists
also.  I've started to work on that too.

Bruce

Danilo Tuler wrote:

> Hi Bruce and everybody,
>
>> By the way, I'm getting along well binding EventLists to JFace
>> TableViewers.
>
> I read other emails regarding your SWT effort.
> A couple of weeks ago I also wrote a TableViewer support implementation.
> It looks similar to your solution...
> I also implemented support for ILazyContentProvider, which provides
> elements on demand. It's working fine.
>
> And I started to develop a RCP IssuesBrower.
> This could be a nice testbed for the SWT support.
>
> You can try to join efforts.
> Is your code available in any CVS or SVN repository?
>
> Finally I thank you guys for the great work on GL.
> It's one of the finest piece of software I've used so far.
> Let's give some attention to the SWT support, please! :-)
>
> Thanks,
> Danilo
>
> ps: I'm also very interested in the TreeViewer support, but I think we
> must focus on the table first.
>
> ps2: I should have answered the other thread, but I just signed the
> dev list, and didn't know how to reply that. sorry.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]



Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

Bruce Alspaugh-2
Your email arrived about the time I hit the send button on the last
one.  I'll take a look a your comments and see if I can incorporate
them.  If you don't mind helping me clean up the EventContentProvider
ListEvent handling the way it should be, I would really appreciate it.

Bruce

James Lemieux wrote:

> Bruce,
>
>    A few comments based on the source code you sent. They're meant to
> be constructive criticism about the code.
>
>
> 1. You have notes to this effect in your EventContentProvider:
>                          /**
>                          * Note: If I knew the index where where the
> insertion or
>                          * deletion is taking place, I could optimise
> the refresh
>                          * rather than redrawing the entire table.
>                          */
>                             case ListEvent.INSERT:
>                             case ListEvent.DELETE:
>                                 tableViewer.refresh();
>                                 break;
>                             case ListEvent.UPDATE :
>                         /**
>                          * Note:  If I knew the property being
> changed, I could
>                          * optimize the update rather than redraw the
> entire row.
>                          */
>                                
> tableViewer.update(swtSource.get(aListEvent.getIndex()), null);
>                                 break;
>
> and I wanted to let you know:
>
> a) in the case of an inserts and deletes you *do* have the index of
> the insertion and deletion (it's available via aListEvent.getIndex(),
> just like you're using for updates)
>
> b) Glazed Lists may not ever be able to furnish you with the property
> change so you can redraw just a cell rather than the entire row. In
> Swing, we also live with that limitation, though in practice the
> performance benefits are generally quite negligible for about 99% of
> all tables.
>
> 2. This is an important fix: EventContentProvider.listChanged(...)
> should *NOT* acquire the read lock for the swtSource. We've removed
> the acquisition of that lock from our swing EventTableModel recently
> because it can cause deadlock and it is unnecessary.
>
> Specifically, it caused deadlock in 1.7.0 because we changed the
> EventSelectionModel to correctly start acquiring the writeLock when
> changes were made to it. That change created a situation where the
> Swing EDT would deadlock *itself* by first acquiring the readLock() in
> the EventTableModel and then later try to acquire the writeLock() from
> the same ReadWriteLock pair.
>
> The reason that the lock is not needed is that we're reacting to a
> ListEvent there, implying that some Thread somewhere has made a change
> to the list, which in turn implies that Thread should be holding the
> writeLock() *already* if concurrent access is a problem for them. And
> if concurrent access isn't a problem then the readLock() *still* is
> unnecessary. Either way, the acquisition of the readLock in the
> listChanged() method is superfluous or plain wrong.
>
> 3. You're using an antipattern in the TableViewerManager constructor
> that has bitten me in the ass in the past. Specifically, you're using
> local factory methods to produce reasonable implementations of some
> JFace interfaces, like so:
>
> tableViewer.setContentProvider(getContentProvider());
> tableViewer.setLabelProvider(getLabelProvider());
>
> Right now, it seems like a great idea because you've factored out two
> methods that could be overridden by subclasses, should they need to
> produce a "custom" content provider or label provider. But, there is a
> nasty problem with calling those methods from the constructor of the
> super class. The implied assumption in getContentProvider() and
> getLabelProvider() is that they don't require *any* member variable
> state (object state) in order to function. To put it more concretely,
> imagine I write something like this (even if it is a contrived bogus
> example):
>
> public class MyCustomTableViewerManager extends TableViewerManager {
>
>     private Object[] contentProviderElements;
>
>     public MyCustomTableViewerManager(TableViewer aTableViewer,
> EventList aSourceList, TableFormat aTableFormat, Object[]
> contentProviderElements) {
>
>        // this is going to call our definition of getContentProvider()
>        super(aTableViewer, aSourceList, aTableFormat);
>
>        this.contentProviderElements = contentProviderElements;
>     }
>
>     // I want a custom content provider, so I'm overriding
>     public IStructuredContentProvider getContentProvider() {
>
>         // when this line executes, the implied reference to the
> enclosing class is null!
>         // i.e. MyCustomTableViewerManager.this == null
>         return new IStructuredContentProvider() {
>             public Object[] getElements(Object aInputElements) {
>                 // I want to return the same Object[] each time, no
> matter what
>                 return contentProviderElements;
>             }
>
>             public void inputChanged(Viewer aViewer, Object aOldInput,
> Object aNewInput) {
>                 // this should never happen - no-op implementation
>             }
>         }
>     }
> }
>
> One of the best ways to combat the antipattern is simply to ask for
> the objects you need in the constructor, rather than using "local
> factory methods". It means callers have to do a bit more work, but it
> is work "on their own terms", rather than work to be done "at the time
> and location determined by the superclass." Overloading the
> constructor to include at least one "uber long and very explicit list
> of parameters" is probably something to consider, with the idea that
> about 95% of all API users will be able to get by with one of the
> smaller constructors anyway.
>
> 4. Regarding EventLabelProvider, if you think there are valid cases to
> extend that class, by all means expose it as a public class. If,
> however, you suspect there aren't many extension points, and you'd
> rather be conservative at first, we can throw it behind a factory
> method on GlazedListsSWT, and move EventLabelProvider into the impl
> package. (without knowing anything about expected usage, I'd probably
> start with the latter)
>
> James
>
> On 9/1/06, *Bruce Alspaugh* < [hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I've attached what I have written so far.  It's very preliminary and
>     needs more testing.
>
>     There are a lot of bug fixes, backports, and API construction work
>     going
>     on in JFace right now.  Hopefully, by Eclipse 3.3 JFace will be much
>     more stable.  I've stayed away from using virtual tables because of
>     known bugs like:  
>     https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755
>     <https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755>
>
>     I'm not sure the best way to submit the JFace extension that builds on
>     the SWT extension.  I need two more packages:
>     ca.odell.glazedlists.jface and ca.odell.glazedlists.impl.jface.  The
>     second one would be tagged as "x-internal:=true" in the export
>     package
>     list.  I also need to add one more dependency to the manifest.mf so it
>     would read:
>
>     "Require-Bundle: org.eclipse.swt, org.eclipse.jface,
>     org.eclipse.core.runtime"
>
>     We need to support a plain ListViewer and the various checkbox lists
>     also.  I've started to work on that too.
>
>     Bruce
>
>     Danilo Tuler wrote:
>     > Hi Bruce and everybody,
>     >
>     >> By the way, I'm getting along well binding EventLists to JFace
>     >> TableViewers.
>     >
>     > I read other emails regarding your SWT effort.
>     > A couple of weeks ago I also wrote a TableViewer support
>     implementation.
>     > It looks similar to your solution...
>     > I also implemented support for ILazyContentProvider, which provides
>     > elements on demand. It's working fine.
>     >
>     > And I started to develop a RCP IssuesBrower.
>     > This could be a nice testbed for the SWT support.
>     >
>     > You can try to join efforts.
>     > Is your code available in any CVS or SVN repository?
>     >
>     > Finally I thank you guys for the great work on GL.
>     > It's one of the finest piece of software I've used so far.
>     > Let's give some attention to the SWT support, please! :-)
>     >
>     > Thanks,
>     > Danilo
>     >
>     > ps: I'm also very interested in the TreeViewer support, but I
>     think we
>     > must focus on the table first.
>     >
>     > ps2: I should have answered the other thread, but I just signed the
>     > dev list, and didn't know how to reply that. sorry.
>     >
>     >
>     ---------------------------------------------------------------------
>     > To unsubscribe, e-mail: [hidden email]
>     <mailto:[hidden email]>
>     > For additional commands, e-mail:
>     [hidden email]
>     <mailto:[hidden email]>
>     >
>     >
>
>
>
>     ---------------------------------------------------------------------
>     To unsubscribe, e-mail: [hidden email]
>     <mailto:[hidden email]>
>     For additional commands, e-mail: [hidden email]
>     <mailto:[hidden email]>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Hierarchical Glazed Lists

James Lemieux
PMD actually checks for the case of constructors calling overridable methods. For a better explanation of the antipattern I mentioned, check out the PMD rule called ConstructorCallsOverridableMethod here http://pmd.sourceforge.net/rules/design.html, or google something like:

"calling overridable methods from constructors"

to get a long list of websites that offer the explanation on why it's a bad idea.

James

On 9/1/06, Bruce Alspaugh <[hidden email]> wrote:
Your email arrived about the time I hit the send button on the last
one.  I'll take a look a your comments and see if I can incorporate
them.  If you don't mind helping me clean up the EventContentProvider
ListEvent handling the way it should be, I would really appreciate it.

Bruce

James Lemieux wrote:

> Bruce,
>
>    A few comments based on the source code you sent. They're meant to
> be constructive criticism about the code.
>
>
> 1. You have notes to this effect in your EventContentProvider:
>                          /**
>                          * Note: If I knew the index where where the
> insertion or
>                          * deletion is taking place, I could optimise
> the refresh
>                          * rather than redrawing the entire table.
>                          */
>                             case ListEvent.INSERT:
>                             case ListEvent.DELETE :
>                                 tableViewer.refresh();
>                                 break;
>                             case ListEvent.UPDATE :
>                         /**
>                          * Note:  If I knew the property being
> changed, I could
>                          * optimize the update rather than redraw the
> entire row.
>                          */
>
> tableViewer.update(swtSource.get(aListEvent.getIndex ()), null);
>                                 break;
>
> and I wanted to let you know:
>
> a) in the case of an inserts and deletes you *do* have the index of
> the insertion and deletion (it's available via aListEvent.getIndex(),
> just like you're using for updates)
>
> b) Glazed Lists may not ever be able to furnish you with the property
> change so you can redraw just a cell rather than the entire row. In
> Swing, we also live with that limitation, though in practice the
> performance benefits are generally quite negligible for about 99% of
> all tables.
>
> 2. This is an important fix: EventContentProvider.listChanged (...)
> should *NOT* acquire the read lock for the swtSource. We've removed
> the acquisition of that lock from our swing EventTableModel recently
> because it can cause deadlock and it is unnecessary.
>

> Specifically, it caused deadlock in 1.7.0 because we changed the
> EventSelectionModel to correctly start acquiring the writeLock when
> changes were made to it. That change created a situation where the
> Swing EDT would deadlock *itself* by first acquiring the readLock() in
> the EventTableModel and then later try to acquire the writeLock() from
> the same ReadWriteLock pair.
>
> The reason that the lock is not needed is that we're reacting to a
> ListEvent there, implying that some Thread somewhere has made a change
> to the list, which in turn implies that Thread should be holding the
> writeLock() *already* if concurrent access is a problem for them. And
> if concurrent access isn't a problem then the readLock() *still* is
> unnecessary. Either way, the acquisition of the readLock in the
> listChanged() method is superfluous or plain wrong.
>
> 3. You're using an antipattern in the TableViewerManager constructor
> that has bitten me in the ass in the past. Specifically, you're using
> local factory methods to produce reasonable implementations of some
> JFace interfaces, like so:
>
> tableViewer.setContentProvider (getContentProvider());
> tableViewer.setLabelProvider(getLabelProvider());
>
> Right now, it seems like a great idea because you've factored out two
> methods that could be overridden by subclasses, should they need to
> produce a "custom" content provider or label provider. But, there is a
> nasty problem with calling those methods from the constructor of the
> super class. The implied assumption in getContentProvider() and
> getLabelProvider() is that they don't require *any* member variable
> state (object state) in order to function. To put it more concretely,
> imagine I write something like this (even if it is a contrived bogus
> example):
>
> public class MyCustomTableViewerManager extends TableViewerManager {
>
>     private Object[] contentProviderElements;
>
>     public MyCustomTableViewerManager(TableViewer aTableViewer,
> EventList aSourceList, TableFormat aTableFormat, Object[]
> contentProviderElements) {
>
>        // this is going to call our definition of getContentProvider()
>        super(aTableViewer, aSourceList, aTableFormat);
>
>        this.contentProviderElements = contentProviderElements;
>     }
>
>     // I want a custom content provider, so I'm overriding
>     public IStructuredContentProvider getContentProvider() {
>
>         // when this line executes, the implied reference to the
> enclosing class is null!
>         // i.e. MyCustomTableViewerManager.this == null
>         return new IStructuredContentProvider() {
>             public Object[] getElements(Object aInputElements) {
>                 // I want to return the same Object[] each time, no
> matter what
>                 return contentProviderElements;
>             }
>
>             public void inputChanged(Viewer aViewer, Object aOldInput,
> Object aNewInput) {
>                 // this should never happen - no-op implementation
>             }
>         }
>     }
> }
>
> One of the best ways to combat the antipattern is simply to ask for
> the objects you need in the constructor, rather than using "local
> factory methods". It means callers have to do a bit more work, but it
> is work "on their own terms", rather than work to be done "at the time
> and location determined by the superclass." Overloading the
> constructor to include at least one "uber long and very explicit list
> of parameters" is probably something to consider, with the idea that
> about 95% of all API users will be able to get by with one of the
> smaller constructors anyway.
>
> 4. Regarding EventLabelProvider, if you think there are valid cases to
> extend that class, by all means expose it as a public class. If,
> however, you suspect there aren't many extension points, and you'd
> rather be conservative at first, we can throw it behind a factory
> method on GlazedListsSWT, and move EventLabelProvider into the impl
> package. (without knowing anything about expected usage, I'd probably
> start with the latter)
>
> James
>
> On 9/1/06, *Bruce Alspaugh* < [hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I've attached what I have written so far.  It's very preliminary and
>     needs more testing.
>
>     There are a lot of bug fixes, backports, and API construction work
>     going
>     on in JFace right now.  Hopefully, by Eclipse 3.3 JFace will be much
>     more stable.  I've stayed away from using virtual tables because of

>     known bugs like:
>     https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755
>     <https://bugs.eclipse.org/bugs/show_bug.cgi?id=154755>
>
>     I'm not sure the best way to submit the JFace extension that builds on
>     the SWT extension.  I need two more packages:
>     ca.odell.glazedlists.jface and ca.odell.glazedlists.impl.jface.  The
>     second one would be tagged as "x-internal:=true" in the export
>     package
>     list.  I also need to add one more dependency to the manifest.mf so it
>     would read:
>
>     "Require-Bundle: org.eclipse.swt, org.eclipse.jface,
>     org.eclipse.core.runtime "
>
>     We need to support a plain ListViewer and the various checkbox lists
>     also.  I've started to work on that too.
>
>     Bruce
>
>     Danilo Tuler wrote:
>     > Hi Bruce and everybody,
>     >
>     >> By the way, I'm getting along well binding EventLists to JFace
>     >> TableViewers.
>     >
>     > I read other emails regarding your SWT effort.
>     > A couple of weeks ago I also wrote a TableViewer support
>     implementation.
>     > It looks similar to your solution...
>     > I also implemented support for ILazyContentProvider, which provides
>     > elements on demand. It's working fine.
>     >
>     > And I started to develop a RCP IssuesBrower.
>     > This could be a nice testbed for the SWT support.
>     >
>     > You can try to join efforts.

>     > Is your code available in any CVS or SVN repository?
>     >
>     > Finally I thank you guys for the great work on GL.
>     > It's one of the finest piece of software I've used so far.
>     > Let's give some attention to the SWT support, please! :-)
>     >
>     > Thanks,
>     > Danilo
>     >
>     > ps: I'm also very interested in the TreeViewer support, but I
>     think we
>     > must focus on the table first.
>     >
>     > ps2: I should have answered the other thread, but I just signed the
>     > dev list, and didn't know how to reply that. sorry.
>     >
>     >
>     ---------------------------------------------------------------------
>     > To unsubscribe, e-mail: [hidden email]
>     <mailto:[hidden email]>
>     > For additional commands, e-mail:
>     [hidden email]
>     <mailto:[hidden email]>
>     >
>     >
>
>
>
>     ---------------------------------------------------------------------
>     To unsubscribe, e-mail: [hidden email]
>     <mailto:[hidden email]>
>     For additional commands, e-mail: [hidden email]
>     <mailto:[hidden email]>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]


Reply | Threaded
Open this post in threaded view
|

RE: Hierarchical Glazed Lists

Danilo Tuler-2
In reply to this post by James Lemieux

> Overloading the constructor to include at least one "uber long
> and very explicit list of parameters" is probably something to
> consider, with the idea that about 95% of all API users will be
> able to get by with one of the smaller constructors anyway.

I agree.

> 4. Regarding EventLabelProvider, if you think there are valid cases
> to extend that class, by all means expose it as a public class.
> If, however, you suspect there aren't many extension points, and
> you'd rather be conservative at first, we can throw it behind a
> factory method on GlazedListsSWT, and move EventLabelProvider into
> the impl package. (without knowing anything about expected usage,
> I'd probably start with the latter)

I disagree. I do think this kind of class should be public.
I see use cases where the user would subclass it.
This happened with me with a class from the KTable support.
I don't think the users will be confused.

-- Danilo

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]