Community
Participate
Working Groups
Currently the TableViewerDescription requires a converter for any non String property. This is an overkill, as the IDataBindingContext has many default conversion factories (e.g., Integer to String) in addition to potentially user added support factories I have changed the code so that a TableViewerDescription’s column now has a new property-type property. If a converter is not given, it will this type designation to create a convertor from the binding context. The last thing that remain is to change the add() API to optionaly include it. Right now (in my test code) I do some (ugly) thing like: desc.addColumn("numberOfPersons","numberOfPersons"); // Joe need to fix this desc.getColumn(1).setPropertyType(Integer.class); // This defect is all about this Joe, you were going to fix the add() api to remove the header’s text. We need to also figure out a way to support the property type (vs. a converter) without exploding/verbosing the API. There is another option: rather than forcing the user to specify the property’s type, the TableViewerUpdatableCollectionExtended can lazily resolve the converter/type, the first time it needs a convert, at this time he is guaranteed to have a valid instance.
I vote for explicit declaration of the property type for a column, for consistency with the rest of the API and to enable early error detection.
The change to remove the text is comitted. There are three API methods that are variants of desc.addColumn("firstName") desc.addColumn(0,"firstName") that include the cellEditor, validator and converter Gili - For the property type are suggesting we remove the ability to specify a converter ? Or that we allow a property type to be used to derive one, and using discovery we can look at the expected type of the collection and property to work out a default one AND have the ability to specify an explicit property type ? I suppose the explicit one would be used if the collection's contents were typed to an interface or abstract class and the developer had more precise knowledge about what the actual type was going to be. One other thing I'd like to suggest is that we change the String to be Object, so you can do stuff like desc.addColumn(EStructuralFeature) or desc.addColumn(java.lang.reflect.Field) or whatever. Basically having a String as the column identifier means it has to always be a String but making it Object means it can be anything that the factory can turn into a column such as an EMF feature of java.lang.reflect thing or java.beans.PropertyDescriptor or whatever.
I don't like having the explicit declaring of the type for each column. This makes the API have to be something like desc.addColumn("firstName",String.class); and all of the other flavors where the column index and validator, converter, cell editor can be specified on the method arguments. However we know during the Bind what the collection is going to be typed to. I think Gili's idea of making it a lazy lookup of the type is a cleaner API.
We only need the type for looking up a converter or validator, so when these are given, the type is not needed. I admit that passing in the type is a bit verbose, but it enables early error reporting. I believe that doing it lazily would lead to error cases that are difficult to debug/understand.
I think the important question to ask here is "are there any use-cases that we can't support if we specify the data types early"?
So is the only benefit of having to specify a single type to avoid having to specify a converter and validator so desc.addColumn("price",new DoubleCellEditor(), new DoubleConverter(),new DoubleValidator()); becomes desc.addColumn("price",Double.TYPE); so that factory lookups can provide default cell editor, converter and validator ? Other than that there is no other need for anyone else to read the type ? so it could basically be a public setter and private getter ?
(In reply to comment #5) > I think the important question to ask here is "are there any use-cases that we > can't support if we specify the data types early"? We can support all use cases either way. The question is one of having a simpler API (the datatype inferred at runtime from the collection's contents' type) versus an API where the datatype must always be specified for any property type other than String. I think the simplest API should be one we should always veer towards unless there is good reason to not do so. Boris' reason is that having more information specified up front catches errors earlier is a good one, however the error catching can only ever be done by verifying the type of the collection against the explicit type of the specified property. Gili's argument is strong to me, which is that we always use the calculated type of the collection's column UNLESS it has been explicitly overridden, rather than have an API that seems to unnecessarily check and force the user to have advance knowledge of something the framework can infer.
(In reply to comment #7) > always use the calculated type of > the collection's column UNLESS it has been explicitly overridden, rather than > have an API that seems to unnecessarily check and force the user to have > advance knowledge of something the framework can infer. This makes sense to me and seems like a reasonable compromise. Thoughts, Boris?
> always use the calculated type of > the collection's column UNLESS it has been explicitly overridden, rather than > have an API that seems to unnecessarily check and force the user to have > advance knowledge of something the framework can infer. I'm still not convinced that lazyness is a good idea for this, but since all of you seem to agree, I'll consider myself overruled. Please write Javadoc for the methods to make sure that the behaviour can be explained.
Defaulting of validator, converter and cell editor now occurs based on the type of the first element added to the collection. TablesScenarios has a test to check this (testScenario07) so there is no need for code like desc.getColumn(1).setPropertyType(Integer.class); It is still possible to specify an explicit type that takes precedence over the one from the first element, which might occur if for example the element type's property was typed to an interface yet you knew it was going to be a specific implementation class
Reopening since only two of the addColumn() methods have Javadoc.
*** Bug 122568 has been marked as a duplicate of this bug. ***
Here's what Scott wrote in bug 122568 about this: ---- I'm currently attempting to build some reusable logic that builds nested trees of IUpdatableValues to easily bind to leaves deep in a tree of objects (patch on that later). So you wind up with String property descriptions like of "foo.bar.name" that binds to the getName() method of an object returned by getBar() on a parent object returned by getFoo() on a root object. The issue with this is that the anonymous IUpdatableFactory in the DataBindingContext.registerFactories() method expects a type to be specified. I would argue that in most cases the objects being bound to don't exist until after bind time. This is especially true if your root object is really an IUpdatableValue serving as a selection holder. In such a case you could be binding to a whole tree of "thin air" until the selection is populated. I'm guessing the argument for specific types is that validators and converters need to be looked up. I would counter that these could be lazily retrieved when actual object instances exist to convert. Further, I think there are times the very values you are attempting to bind to might be polymorphic in nature. In this case the converters would have to be looked up dynamically anyway. For example ... say you have a collection of objects mapped to a table. Each object has a getType() method that returns type Object. The concrete class being returned may vary. You would need a converter to create a value to represent this object in a cell of the table. In this case the type itself is polymorphic and a converter can't simply be retrieved once.
Sorry, but I don't have time left in 3.2 to address this.
*** This bug has been marked as a duplicate of 121127 ***
