Bug 57273 - I need to add TreeModelEvent to ViewModelAPI
I need to add TreeModelEvent to ViewModelAPI
Status: CLOSED FIXED
Product: debugger
Classification: Unclassified
Component: Code
4.x
All All
: P2 (vote)
: 4.x
Assigned To: apireviews
issues@debugger
: API, API_REVIEW_FAST, PERFORMANCE
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-03-31 16:07 UTC by Jan Jancura
Modified: 2010-04-29 09:21 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
diff (5.70 KB, patch)
2005-03-31 16:21 UTC, Jan Jancura
Details | Diff
New diff. (107.14 KB, patch)
2005-04-01 14:02 UTC, Jan Jancura
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Jancura 2005-03-31 16:07:18 UTC
ViewModelAPI currently supports two types of changes only:
1) all tree is changed
2) node and all subnodes are changed

We need more fine grained firing. This change has been already discussed on
DevRev - see http://jupiter.czech.sun.com/wiki/view/Devrev/DevRevMeeting050118.
Comment 1 Jan Jancura 2005-03-31 16:21:05 UTC
Created attachment 21272 [details]
diff
Comment 2 Jan Jancura 2005-03-31 16:22:15 UTC
apichanges updated, version updated
Comment 3 Tomas Pavek 2005-04-01 07:44:18 UTC
Looks good, I agree with the change.
Comment 4 Maros Sandor 2005-04-01 08:12:34 UTC
It would be nice to also attach this TreeModelEvent class you are adding.
Comment 5 Miloslav Metelka 2005-04-01 08:23:40 UTC
I would personally prefer to only have the method(s) having the TreeModelEvent
as a parameter in the TreeModelListener.
Also consider whether you really want to have:

public void treeNodeChanged (Object node);
public void treeNodeChanged (TreeModelEvent event);

In case you accidentally not pass the TreeModelEvent parameter although you
originally wanted to, the other version of the method with Object parameter will
be used and you may not know about it.

Just cosmetic: should not the method be named "treeModelChanged" instead of
"treeNodeChanged" once you have TreeModelListener and TreeModelEvent?

I do not see the TreeModelEvent source in the diff, have you made "cvs add" and
"cvs diff -N" for it?
Comment 6 Jan Jancura 2005-04-01 14:01:18 UTC
I have applied suggested changes:

1) removed methods:
public void treeNodeChanged (Object node);
public void treeNodeChanged (TreeModelEvent event);

2) I have created three new innerclasses to cover functionality of this removed
methods:
ModelEvent.TreeChanged
ModelEvent.NodeChanged
ModelEvent.TableValueChanged

3) I have renamed TreeModelListener & TreemodelEvent to ModelListeenr and
ModelEvent. This two classes notifies about changes in more than TreeModel.

Comment 7 Jan Jancura 2005-04-01 14:02:52 UTC
Created attachment 21294 [details]
New diff.
Comment 8 Maros Sandor 2005-04-01 14:57:31 UTC
Large change it is, hard to consider overall impact. Diff mostly consists of
simple method parameter renames and other straightforward changes and seems ok
to me.
Comment 9 Miloslav Metelka 2005-04-01 15:10:22 UTC
Although it's sort of uncommon to subclass the event class I agree with the change.
Comment 10 _ rkubacki 2005-04-01 17:35:51 UTC
re event class subclassing: what about AWTEvent? does not look so uncommon
Comment 11 _ rkubacki 2005-04-01 18:47:08 UTC
It is excellent that you have already commited the change. Everybody had a
chance to review what you are going to do just before entering high resistence.

Did you tried to read what documentation you wrote?

+    /**
+     * All three has been changed event.
+     */
+    public static class TreeChanged extends ModelEvent {

Should there be a @since tag? Or is it just a stupid request as your API is
still rapidly evolving and noone cares? The description of API change is
comletely wrong too. Anyone who will read the documentation will have a problem.
Do not use any URLs reachable within Sun intranet in issuezilla and in your
documentation. And what was discussed?

#  JJ: Good, but I have another incompatible change: There are only changeall,
changesubstree, we need changesomethingfinegrained - add TreeModelEvent
# JJ: I will change all known modules (including JSP)
# JT: Announce on nbdev@

What has improved wrt performance after your commit? I did not find any fine
grained handling of your new events.
Comment 12 Jan Jancura 2005-04-01 22:43:01 UTC
Sorry Radime, but I do not like style of your coments. Be more polite next time,
please.

1) I do not see any reason why subclass AWTEvent. It contains methods like
consume (), getID (). And its not applicable to ModelEvent, I think. but we can
discuss it.

2) I have commited this changes according to our processes, as far as I know.
Change has been reviewed by DevRev team. It has been suggested by DevRev, and
clients of this API, so I do not understand your point.

3) Performance: This commit has opened the door for future performance
imporvements. Thats why the PERFORMANCE tag was there. I wanted to do as small
as posible change, so implementation inside of the ViewModel has not been
changed. We can discuss it offline, if you have some performance requirements.
Or you can fire a bug for it, and we will decide if it should be fixed for nb41,
or if it should be waived.

I do not want to comment the rest of your sentences here.
Comment 13 _ rkubacki 2005-04-04 21:32:56 UTC
re 1) - I responded to Mila's objection that it is uncommon to subclass events

re 2) - I did not not attend meeting where the change was discussed and form
meeting minutes + this issue it is not clear to me why this change is so
important that it needs to be done so quickly. I would like to do my best to
contribute to this review but it is hard. You have sent a large diff without
explanation so only limited number of people had a chance to comment it. You put
it to your API clients without giviing them a real chance to influence the
change. Strange as you had 6 weeks but waited to last days before high
resistance.   

It still affirm that the resulting documentation has a low quality. It needs to
be fixed and improved. Maybe that you've followed the process but it does not
guarantee the result yet.

re 3) - that's fine. I had false expectation that it actually improves event
handling. Even if it is not current state I am pleased that we will have API
that will not block us from developing fast the IDE.
Comment 14 Jan Jancura 2005-04-05 08:06:51 UTC
re 2) 
This issue has been originally recognized as P3 priority. We have too many
criticall P1 issues, so I have not time to fix it. On the other hand, we do not
want to do any incompatible changes of ViewModeAPI in next releases, so it was
the last change to fix it. I would prefer to do things sooner, but in this
concrete example it was not possible.

Documentation will be improved. I have send detailed "how to update on new
version" to nbdev - it should be more descriptive for you.



Comment 15 Jan Jancura 2005-04-06 13:39:37 UTC
documentation updated
since added.
Comment 16 Jaroslav Tulach 2005-04-15 08:25:16 UTC
Hanzi, as I notified you a week ago, you change is not yet done: You forget to 
patch the signature test files for 4.0 and every day that is at least one 
email sent to api-changes alias. See: 
http://www.netbeans.org/servlets/ReadMsg?list=api-changes&msgNo=1327&raw=true 
 
You have to fix it. Please talk to Rudolf Balada and help him to fake the 
signature file. 
Comment 17 Jan Jancura 2005-04-15 08:44:40 UTC
I have already ask RE to do this seveal days before.
Current process is stupid. Can you change it, Jarda? Can we (or you at least +
DevRev) gain access to these golden files? Its really strange to call somewhere
if you need to change some API ( ~ open source and such things...).
Comment 18 rbalada 2005-04-15 12:37:15 UTC
Hanzi,

I'm sorry for the delay. I'll do that asap.

Just technical notes. The infrastructure used for the signature test is not
public and is available only to JCP.org members under respective license. That's
the reason the signature (golden) files cannot be made available in netbeans.org
CVS.
Practically you have read only access to those files on RE's internal storage
server, where you can get NetBeans builds. Look for file
netbeans/4.1/internaldata/goldenfiles/nb_spi_nb40.sig
Comment 19 rbalada 2005-04-15 18:05:59 UTC
Hanzi,

Robert has removed CLASS definition from NB 4.0 signature file and committed the
change.

Please note, that there are still class methods refering that class in
parameters clause. This may cause further api-changes notifications. Let us know
if that happens.
Comment 20 Quality Engineering 2010-04-29 09:21:52 UTC
Verified ... and Closing all issues resolved into NetBeans 6.7 and earlier.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo