Re: TableItemConfigurer should be a parameter for EventTableViewer

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

Re: TableItemConfigurer should be a parameter for EventTableViewer

Holger
Ok, I see your point.
I'd suggest to

1.) add the constructor
public EventTableViewer(
  EventList<E> source,
  Table table,
  TableFormat<? super E> tableFormat,
  TableItemConfigurer tableItemConfigurer)

2.) and deprecate  the constructor
public EventTableViewer(
  EventList<E> source,
  Table table,
  String[] propertyNames,
  String[] columnLabels)

because it doesn't add much value.
One could argue that it's "convenient", but I prefer a tight API.
EventTableViewer should not be responsible for creating TableFormats,
let the user do it, for example by using the convenient GlazedLists-factory methods.

James, if you have no objections, I will make these changes...

Holger

>
> When creating the EventTableViewer on a table it starts to render all items
> with the default TableItemConfigurer. When you set a new TableItemConfigurer
> after creating the EventTableViewer it is rendered twice. I propose to
> introduce 2 new constructors where you can supply your TableItemConfigurer
> right from the beginning. It would not break existing code. The 2 already
> defined ctors could call the new ctors with the TableItemConfigurer.DEFAULT
> argument.
>
> public EventTableViewer(EventList<E> source, Table table, TableFormat<?
> super E> tableFormat, TableItemConfigurer tableItemConfigurer) {
>   this.tableItemConfigurer = tableItemConfigurer;
> }
>
> public EventTableViewer(EventList<E> source, Table table, TableFormat<?
> super E> tableFormat) {
>     this(source,table,tableFormat, TableItemConfigurer.DEFAULT);
> }
>
> public EventTableViewer(EventList<E> source, Table table, String[]
> propertyNames, String[] columnLabels, TableItemConfigurer
> tableItemConfigurer) {
>   this(source, table, (TableFormat<E>)GlazedLists.tableFormat(propertyNames,
> columnLabels), tableItemConfigurer);
> }
> --
> View this message in context: http://www.nabble.com/TableItemConfigurer-should-be-a-parameter-for-EventTableViewer-tp20717429p20717429.html
> Sent from the GlazedLists - Dev mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


____________________________________________________________________
Psssst! Schon vom neuen WEB.DE MultiMessenger gehört?
Der kann`s mit allen: http://www.produkte.web.de/messenger/?did=3123


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

Reply | Threaded
Open this post in threaded view
|

Re: TableItemConfigurer should be a parameter for EventTableViewer

James Lemieux
No objections here. Go right ahead Holger.

James

On Sat, Dec 13, 2008 at 7:34 AM, Holger Brands <[hidden email]> wrote:
Ok, I see your point.
I'd suggest to

1.) add the constructor
public EventTableViewer(
 EventList<E> source,
 Table table,
 TableFormat<? super E> tableFormat,
 TableItemConfigurer tableItemConfigurer)

2.) and deprecate  the constructor
public EventTableViewer(
 EventList<E> source,
 Table table,
 String[] propertyNames,
 String[] columnLabels)

because it doesn't add much value.
One could argue that it's "convenient", but I prefer a tight API.
EventTableViewer should not be responsible for creating TableFormats,
let the user do it, for example by using the convenient GlazedLists-factory methods.

James, if you have no objections, I will make these changes...

Holger

>
> When creating the EventTableViewer on a table it starts to render all items
> with the default TableItemConfigurer. When you set a new TableItemConfigurer
> after creating the EventTableViewer it is rendered twice. I propose to
> introduce 2 new constructors where you can supply your TableItemConfigurer
> right from the beginning. It would not break existing code. The 2 already
> defined ctors could call the new ctors with the TableItemConfigurer.DEFAULT
> argument.
>
> public EventTableViewer(EventList<E> source, Table table, TableFormat<?
> super E> tableFormat, TableItemConfigurer tableItemConfigurer) {
>   this.tableItemConfigurer = tableItemConfigurer;
> }
>
> public EventTableViewer(EventList<E> source, Table table, TableFormat<?
> super E> tableFormat) {
>     this(source,table,tableFormat, TableItemConfigurer.DEFAULT);
> }
>
> public EventTableViewer(EventList<E> source, Table table, String[]
> propertyNames, String[] columnLabels, TableItemConfigurer
> tableItemConfigurer) {
>   this(source, table, (TableFormat<E>)GlazedLists.tableFormat(propertyNames,
> columnLabels), tableItemConfigurer);
> }
> --
> View this message in context: http://www.nabble.com/TableItemConfigurer-should-be-a-parameter-for-EventTableViewer-tp20717429p20717429.html
> Sent from the GlazedLists - Dev mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


____________________________________________________________________
Psssst! Schon vom neuen WEB.DE MultiMessenger gehört?
Der kann`s mit allen: http://www.produkte.web.de/messenger/?did=3123


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


Reply | Threaded
Open this post in threaded view
|

Re: TableItemConfigurer should be a parameter for EventTableViewer

Holger
In reply to this post by Holger
I just checked in these changes.
They will be available with the next latest build.

Holger

> Ok, I see your point.
> I'd suggest to
>
> 1.) add the constructor
> public EventTableViewer(
>   EventList<E> source,
>   Table table,
>   TableFormat<? super E> tableFormat,
>   TableItemConfigurer tableItemConfigurer)
>
> 2.) and deprecate  the constructor
> public EventTableViewer(
>   EventList<E> source,
>   Table table,
>   String[] propertyNames,
>   String[] columnLabels)
>
> because it doesn't add much value.
> One could argue that it's "convenient", but I prefer a tight API.
> EventTableViewer should not be responsible for creating TableFormats,
> let the user do it, for example by using the convenient GlazedLists-factory methods.
>
> James, if you have no objections, I will make these changes...
>
> Holger
>
> >
> > When creating the EventTableViewer on a table it starts to render all items
> > with the default TableItemConfigurer. When you set a new TableItemConfigurer
> > after creating the EventTableViewer it is rendered twice. I propose to
> > introduce 2 new constructors where you can supply your TableItemConfigurer
> > right from the beginning. It would not break existing code. The 2 already
> > defined ctors could call the new ctors with the TableItemConfigurer.DEFAULT
> > argument.
> >
> > public EventTableViewer(EventList<E> source, Table table, TableFormat<?
> > super E> tableFormat, TableItemConfigurer tableItemConfigurer) {
> >   this.tableItemConfigurer = tableItemConfigurer;
> > }
> >
> > public EventTableViewer(EventList<E> source, Table table, TableFormat<?
> > super E> tableFormat) {
> >     this(source,table,tableFormat, TableItemConfigurer.DEFAULT);
> > }
> >
> > public EventTableViewer(EventList<E> source, Table table, String[]
> > propertyNames, String[] columnLabels, TableItemConfigurer
> > tableItemConfigurer) {
> >   this(source, table, (TableFormat<E>)GlazedLists.tableFormat(propertyNames,
> > columnLabels), tableItemConfigurer);
> > }
> > --
> > View this message in context: http://www.nabble.com/TableItemConfigurer-should-be-a-parameter-for-EventTableViewer-tp20717429p20717429.html
> > Sent from the GlazedLists - Dev mailing list archive at Nabble.com.
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
>
>
> ____________________________________________________________________
> Psssst! Schon vom neuen WEB.DE MultiMessenger gehört?
> Der kann`s mit allen: http://www.produkte.web.de/messenger/?did=3123
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


_______________________________________________________________________
Sensationsangebot verlängert: WEB.DE FreeDSL - Telefonanschluss + DSL
für nur 16,37 Euro/mtl.!* http://dsl.web.de/?ac=OM.AD.AD008K15039B7069a


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