Bugzilla – Full Text Bug Listing
| Summary: | Need ability to set arbitrary data on a widget | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Will Smythe <smythew> | ||||||||||
| Component: | EDT | Assignee: | lu lu <lulu> | ||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P2 | CC: | chenzhh, hjiyong, jinfahua, pharmon, svihovec, xiaobinc | ||||||||||
| Version: | unspecified | ||||||||||||
| Target Milestone: | --- | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows XP | ||||||||||||
| Whiteboard: | |||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 365062, 363592 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Will Smythe
The following APIs should be sufficient for this requirement:
function getData() returns (any?)
function getData(key String in) returns (any?)
Null is returned if data has not been set.
function setData(value any? in)
function setData(key String in, value any? in)
Passing null as the value to setData would cause the data to be cleared.
This would allow the developer to do the following:
Example 1
=========
myWidget.setData(someRecord);
...
a MyRecord = myWidget.getData() as MyRecord;
Example 2
=========
myWidget.setData(32);
myWidget.setData("rec", someRecord);
myWidget.setData("arr", someArray);
...
i int = myWidget.getData() as int;
a MyRecord = myWidget.getData("rec")as MyRecord;
b MyRecord[] = myWidget.getData("arr") as MyRecord[];
setData/getData method have been defined as private methods in DojoBase.js, and be used in widgets, like DojoButton.js, DojoComboBox.js and so on, so we should decide which is the better way to implement. 1. Use getUserData/setUserData API name instead of getData/setData which are referred in comment 1. eg. function getUserData() returns (any?); function getUserData(key String in) returns (any?) function setUserData(value any? in); function setUserData(key String in, value any? in) 2. Rename private methods setData/getData in DojoBase.js and all extended widgets which used such methods. What's your opinion? Thanks. And Paul, we should add APIs in ExternalTypes.egl, but we can't compile edtCompiler.eglar successfully, please help to investigate. Thanks. Can you tell me what problem(s) you are having compiling the system parts? I was able to recompile them 2 day ago with no problem. (In reply to comment #2) > setData/getData method have been defined as private methods in DojoBase.js, and > be used in widgets, like DojoButton.js, DojoComboBox.js and so on, so we should > decide which is the better way to implement. > > 1. Use getUserData/setUserData API name instead of getData/setData which are > referred in comment 1. > eg. function getUserData() returns (any?); > function getUserData(key String in) returns (any?) > function setUserData(value any? in); > function setUserData(key String in, value any? in) > > 2. Rename private methods setData/getData in DojoBase.js and all extended > widgets which used such methods. > > What's your opinion? Thanks. The data is not really 'user' data... it's any data the developer needs to associate with a widget for use later in the application. get/setData is the API used by SWT, and this requirement is requesting for EGL widgets to have the identical capability (so, logically it makes sense for us to use the same API names). (In reply to comment #4) > Can you tell me what problem(s) you are having compiling the system parts? I > was able to recompile them 2 day ago with no problem. Hi Paul, I think Lulu has sent you an email about the problem yesterday, can you find that for the description, thanks. OK, I believe it makes sense to take the second option in comment #2 (In reply to comment #5) > (In reply to comment #2) > > setData/getData method have been defined as private methods in DojoBase.js, and > > be used in widgets, like DojoButton.js, DojoComboBox.js and so on, so we should > > decide which is the better way to implement. > > > > 1. Use getUserData/setUserData API name instead of getData/setData which are > > referred in comment 1. > > eg. function getUserData() returns (any?); > > function getUserData(key String in) returns (any?) > > function setUserData(value any? in); > > function setUserData(key String in, value any? in) > > > > 2. Rename private methods setData/getData in DojoBase.js and all extended > > widgets which used such methods. > > > > What's your opinion? Thanks. > > The data is not really 'user' data... it's any data the developer needs to > associate with a widget for use later in the application. get/setData is the > API used by SWT, and this requirement is requesting for EGL widgets to have the > identical capability (so, logically it makes sense for us to use the same API > names). I also have the same requirement when developing an EGL application these days. Currently, I have to extend an existing widget, add getter/setter function to access the data that will be used in event handler function, which is very annoyed. It looks to me that such of data can be thought of "model" of a widget. Not sure if it's OK for us to change the getData to getModel, and setData to setModel? Also, I think removeData(removeModel) & clearData(clearModel) are also required. (In reply to comment #1) > The following APIs should be sufficient for this requirement: > > function getData() returns (any?) > function getData(key String in) returns (any?) > > Null is returned if data has not been set. > > function setData(value any? in) > function setData(key String in, value any? in) > > Passing null as the value to setData would cause the data to be cleared. > > This would allow the developer to do the following: > > Example 1 > ========= > myWidget.setData(someRecord); > > ... > > a MyRecord = myWidget.getData() as MyRecord; > > > Example 2 > ========= > myWidget.setData(32); > myWidget.setData("rec", someRecord); > myWidget.setData("arr", someArray); > > ... > > i int = myWidget.getData() as int; > a MyRecord = myWidget.getData("rec")as MyRecord; > b MyRecord[] = myWidget.getData("arr") as MyRecord[]; I think using 'model' might confuse things with MVC. I have not been following this conversation in much detail, but I would like to throw out a suggestion to see what everyone thinks. We can add a public dictionary to all widgets named 'dataStore', and then users can store multiple pieces of data, by key, and we shouldn't have to introduce any additional methods to the Widget API. All of the normal dictionary operations would be supported, etc. Paul, Looks like an additional annotation was written to the ir when compiling Operation. And this annotation, egl:egl.lang.reflect.mof.operation, could not be instantiated when deserilizing the ir. <operations ID="3" eClass="org.eclipse.edt.mof.egl.Operation" name="$EQ" type="egl:eglx.lang.eboolean" isNullable="false" isStatic="true" isAbstract="false" opSymbol="==" > <annotations ID="4" eClass="egl:egl.lang.reflect.mof.operation" opSymbol="==" /> <annotations ID="5" eClass="dynMof:org.eclipse.edt.mof.egl.Annotation:EGL_Location" len="i:97" off="i:279" line="i:14" /> (In reply to comment #4) > Can you tell me what problem(s) you are having compiling the system parts? I > was able to recompile them 2 day ago with no problem. I agree using "model" would be confusing.
Brian - would this force a new Dictionary object to be created (and in memory) for every widget, since the developer would be able to do something like: myWidget.dataStore.key = "some value"; ? If so, this seems heavy.
So, netting this out .. the primary requirements are:
1) Associate one or more pieces of a data with a widget
2) Have the ability to clear/remove data when no longer needed
I think this can be accomplished with these two APIs:
function getData() returns (any)
function setData(value any in);
If the developer needs to set multiple pieces of data (requirement #1), he/she can create a Dictionary, set the data on it, and then pass this dictionary to the setData method:
d Dictionary {};
d.val1 = someRec;
d.val2 = "some string";
d.val3 = 32.2;
myWidget.setData(d);
-or-
myWidget.setData(2332);
The developer can clear the data (requirement #2) by passing null to the setData method (like SWT).
myWidget.setData(null);
Thoughts?
To avoid having a new dictionary created for every widget, we could override the get/set methods and define the field to initially be null. I had been thinking of this as well, to avoid unnecessary memory consumption. With that said, I re-read this thread and I think I prefer Will's latest proposal in comment 11 over a dictionary. Will, in comment 11 you dropped the ability to set data by name, like in SWT. Was this done because a user can add a Dictionary? Some final questions: * Should the data field be private, with a requirement that the get/set functions be used to store the data, or will the field be public and specify the @EGLProperty annotation? (I think I prefer making the field public and using the EGLProperty annotation) * If we use the EGLProperty annotation, should the functions be made private? I would like to propose the following source: /** * The data field is used to store arbitrary data on a widget. To store * multiple values on a widget at one time, use a Dictionary. To remove * a value stored on a widget, assign null to the data field. **/ data Any?{@EGLProperty}; private function setData(value any? in) if(value == null) data = null; else data = value; end end private function getData()returns(any?) return (data); end NOTE: There is a bug in validation right now that allows a user to pass a null literal value to a non-nullable parameter - I have opened Bug 368725 to track this. NOTE: There is a bug in Java Generation and JS Generation right now that allows a user to pass a null variable value to a non-nullable parameter - I have opened bug 368728 and bug 368729 to track this. Brian - in your response to your question: "Will, in comment 11 you dropped the ability to set data by name, like in SWT. Was this done because a user can add a Dictionary? " The answer is YES ... Can we change the name to getDataStore / setDataStore because getData/setData has already been taken in many widgets such as DataGrid I assume the getData/setData functions in the Dojo widgets have a different purpose, and are considered 'private'? Can we re-name these functions if that is the case? If not, can we expose them as the same get/setData functions on all widgets? (In reply to comment #14) > Can we change the name to > getDataStore / setDataStore > because getData/setData has already been taken in many widgets such as DataGrid Jiyong will take vacation from Jan, 19, so we need to make the decision no later then our tomorrow. The following widgets has data property exposed, and get/setData is private DojoBubbleChart, DojoLineGraph, DojoGrid These widgets can rename the private get/set method The following widgets has data property and get/setData exposed DataGrid We can hide the old get/setData(only access by data property) and replace it with the new function. This will break some of the old application using get/setData function. To rename the existing get/setData function, another problem is that data property has different purpose with get/setData function may be confusing. (In reply to comment #15) > I assume the getData/setData functions in the Dojo widgets have a different > purpose, and are considered 'private'? Can we re-name these functions if that > is the case? If not, can we expose them as the same get/setData functions on > all widgets? DataGrid certainly complicates the situation. I see two solutions: 1) come up with a new API for getting and setting arbitrary data, or 2) stay with get/setData and come up with a new API for setting 'data' on a DataGrid (and DojoGrid). Whatever we do, the solution needs to be consistent across all widgets. I am not opposed to altering the purpose of get/setData on DataGrid, especially if we think there is a more appropriate field we could be using. For example, other widgets use 'value' for the value of the widget. Assuming we think it makes sense, we could introduce a get/setValue on DataGrid. So, would DataGrid.get/setValue be appropriate (for getting/setting the values on the grid) and also consistent with other widgets? If so, I am cool with making this change. Btw, I am not a fan of DataStore because both Dojo and ExtJS (and I'm sure other libraries) use it in the context of setting the values of some widget (like a grid). I don't think I'm a fan of changing the variable names in our grids, since it would almost guarantee a change in behavior for most Rich UI applications. To make sure I'm on the same page, I believe we would need to change the variable name from 'data' to 'value', and also update the function names. Most people probably access the field directly. I was playing around with DojoDialogLib, and I came up with the following 'workaround' that avoids the need for a 'data' field on all widgets (see DojoDialog.zip attachment, and search for "BWS" for the things I changed). I realize that my workaround may be a bit 'heavy handed', but I liked how it removed the need for the 'findParentDialog' and 'buttonClicked' to be defined in DojoDialogLib. It would be even better if I could define the DojoDialogButton inside the same file as DojoDialogLib, and possibly as a 'nested' part within DojoDialogLib itself. Having said that, I think I am still ok with adding a 'data' field to all widgets, but I think we should just pick a name that does not collide with our current names in use. I will propose 'userData'. Created attachment 209643 [details]
DojoDialog Example
userData works for me. Created attachment 209735 [details]
Add get/setUserData method for arbitrary data
Add a property named userData and related get/set methods.
Hi Brian, Can you help to review my changes in attachment? (No @EGLProperty has been used in ExternalTypes, so still use @Property) If I misunderstanding your comments, please help to adjust.
If you agree with my changes, can you help me to check in as I don't have permission to check in, or let Xiao Bin help to check in. Thanks.
How to use it:
d Dictionary{};
d.id = 5;
d.value = "test";
myWidget.setUserData(d);
myWidget.getUserData();
Hi Brian, Any comments about my attached changes? Thanks. I was wrong in my code from comment 12. We don't need to check for null in the setValue function, and can just assign the value to the field. After making that change, I am fine with the Patch. Created attachment 210346 [details]
Modified patch
Thanks Brian. I modify the patch according to your comment 24 and attached it. I don't have the permission to deliver it now, so will require help from another teammate to deliver it. Thanks. Created attachment 210351 [details]
Modified patch
Add some minor changes, and test it again. Thanks.
Comment on attachment 210351 [details]
Modified patch
Looks good. Committed.
Mark this enhancement as resolved. Thanks. I tried various combinations of primitives, dictionaries, arrays, records, etc and every scenario worked. Great new feature! |