This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
The following runtime warning can appear in the log when the user runs a wizard: WARNING - the WizardDescriptor.Panel implementation com.sun.forte4j.webdesigner.client.wizard.fromUDDI.CreateWS ClientUDDIEnterQuerPanel provides itself as the result of getComponent(). This hurts performance and can cause a clash when Component.isValid() is overridden. Please use a separate component class, see details at http://performance.netbeans.org/howto/dialogs/wizard- panels.html. The document pointed to by the above URL asks us to break up our wizard-panels into two classes: one for the WizardDescriptor.Panel, and another for the actual JPanel. I don't want to construct my wizard panels this way, for a couple reasons: 1. Encourages source file proliferation. I prefer the entire Panel definition to be enclosed in a single source file. But to keep it in one file, I would have to make an inner class of the JPanel, and yet another part of Netbeans gives a warning about that (see http://www.netbeans.org/issues/show_bug.cgi?id=29700). 2. Less convenient. It is fairly common for isValid() to access form fields in order to display a helpful message for the user and then indicate, by its return code, whether Next can be enabled. Accessing form fields is most convenient if the wizard panel is also the JPanel. In addition, there is another approach to lazy instantiation of the wizard steps, which I like better, and which doesn't seem to fit the assumptions of this warning. And that is for my wizard iterator code to lazy instantiate its steps when they are accessed by the user. Also, the description in the paragraph in the above URL does not seem to consider the case of more complicated wizards which have branching; for these, we do *not* want Netbeans to instantiate steps in a background thread, even if cpu is available, because we do not want steps in a branch that the user did not choose to be instantiated. Since Netbeans does not manage the step branches, we must manage them ourselves in our wizard framework. For wizards that manage their steps in this way I believe this warning message is inappropriate, and should be removed. For an example of such a wizard, see (closed source): com.sun.forte4j.webdesigner.client.wizard.InvokeWSWizard. I have never seen isValid used on a wizard JPanel, so I don't think the issue with that being an override is significant (I suspect there are ways to code around that if were ever needed). You could call out this possible issue in the WizardDescriptor.Panel.isValid javadoc.
CCing Jesse because AFAIK he's author of comment about isValid clash.
Ok, it seems we have slightly different view of this problem, here are my comments: > 1. Encourages source file proliferation. I prefer the Would it be possible to design UI panel form as outer class and WizardDescriptor.Panel as its inner class? Or am I missing smt here? > 2. Less convenient. True, this comes as natural trade-off for better code separation. Almost any design book I've read recommend separation of UI and non-UI code. > warning. And that is for my wizard iterator code to lazy > instantiate its steps when they are accessed by the user. Yes, but that creates nasty performance problem of slow response when user clicks on Next button. Which is IMO lot worse. > wizards which have branching; for these, we do *not* want > Netbeans to instantiate steps in a background thread, even > if cpu is available, because we do not want steps in a > branch that the user did not choose to be instantiated. Right, good comment. Perhaps I will be able to identify such situation in the code and don't do preloading then. > I have never seen isValid used on a wizard JPanel, so I > don't think the issue with that being an override is > significant That only means that we have been lucky so far. I see the clash as very dangerous because it can produce hard to find bugs. Summary: IMO solving performance/responsiveness issue, avoiding dangerous clash together with better UI/non UI code separation is important enough to keep the warning as it is. (however I'll have to solve the issue with branched wizards when I'll try to preload wizard steps). Is my motivation a bit clearer now?
"Yes, but that creates nasty performance problem of slow response when user clicks on Next button." I think the performance problem here is more theoretical than real. We have many wizards which get this warning, and I have not seen a performance problem in any of them. Dynamic instantiation of a step does not take long. I can point to you some example EE wizards if you'd like to see them (and this warning) in action. "Perhaps I will be able to identify such situation in the code and don't do preloading then" You won't be able to do that with many of our wizards, because you don't have access to the classes that comprise our steps until after our wizard framework (lazily) instantiates them (take a look at com.sun.forte4j.j2ee.lib.wizardFramework.AbstractWizardIterator). "..the clash as very dangerous because it can produce hard to find bugs" Again, I think this is more theoretical than real. Hasn't happened to us, and we've written a lot of wizards. Because it's possible to cause problems, but users are unlikely to run into problems with this (we haven't), I think it if fine to just give a warning about it in the javadoc. How many hard to find bugs have been reported due to this up to now? However many there have been, I think a simple javadoc warning would have been enough to prevent them.
What do you think about this: modify the Netbeans javadoc to say that there are two ways to write wizard steps in Netbeans. There is the recommended way (describe it), and there is another way, which may be more convenient, but has some possible pitfalls you need to avoid (and describe those). To do this, we can create a single writeup (I will help), and link to that writeup from the javadoc for WizardDescriptor, WizardDescriptor.Iterator and WizardDescriptor.Panel.
regarding perf. problem - glad to hear that you don't suffer from this problem, but note that recommended response times are tight - response to Next button click should be really fast, ideally under 100ms, acceptably under 500ms, disaster if over 1 second. I found that some of wizard panels used in core (for example third panel in Tools/Setup Wizard) need more then 500ms to construct themselves, that's why I came up with preloading idea, especially because sometimes it's not even possible to speedup panel construction (complex layout, advanced components like html viewers, many classes loaded). Ok, your suggestion to improve javadoc is reasonable, and I appreciate your will to help me, especially because I'm not native speaker. Could you extend the document mentioned in warning? I'll remove runtime message right now.
great. thanks for fixing this for us. I will put together a draft writeup in the next couple days, and send it to you as an email for review.
fixed in trunk, WizardDescriptor 1.81 -> 1.82.
Created attachment 10031 [details] diff of changes
I modified javadoc to contain links to complete guide how to design wizard steps, including writeup from Pete Eakle (modified to fit structure of document). Just last thing - important! In your writeup, you make assumption that optimizations performed by Netbeans system are equal to lazy loading of wizard steps. Well, no, because system performs *pre-loading* in addition to lazy loading, so responsiveness will be better generally for technique 1. (see guide at http://performance.netbeans.org/howto/dialogs/wizard-panels.html)
Fix is needed in release35 too, please
Yes Andrew, I'm waiting for an approval from release coordinator.
approved for 3.5
Merged also in release35 branch. cvs log: Checking in openide/src/org/openide/WizardDescriptor.java; /cvs/openide/src/org/openide/WizardDescriptor.java,v <-- WizardDescriptor.java new revision: 1.80.4.2; previous revision: 1.80.4.1 done
Thanks all
verified