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.

Bug 138128 - WizardDescriptor magic property names should be Java constants
Summary: WizardDescriptor magic property names should be Java constants
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Dialogs&Wizards (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Milan Kubec
URL: http://bits.netbeans.org/dev/javadoc/...
Keywords: API, API_REVIEW_FAST
: 54905 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-06-24 18:10 UTC by Jesse Glick
Modified: 2008-12-22 11:50 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
openide.dialogs diff (20.31 KB, patch)
2008-06-30 13:48 UTC, Milan Kubec
Details | Diff
java.project diff (8.04 KB, patch)
2008-06-30 13:48 UTC, Milan Kubec
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2008-06-24 18:10:40 UTC
There are a great number of constants described in wizard-guidebook.html, and these constants actually exist in
WizardDescriptor.java - yet they are private! This is unfriendly to code completion and makes it too easy to introduce a
typo.

Please consider the following:

1. Make the constants public, and make sure their Javadoc reflects their usage.

2. Delete wizard-guidebook.html and include any remaining content directly in WD class Javadoc. (Should be small -
general notes about where and when to set properties.)

3. Go through NB sources and replace these property names with references to the new constants.
Comment 1 Milan Kubec 2008-06-30 13:45:46 UTC
*** Issue 54905 has been marked as a duplicate of this issue. ***
Comment 2 Milan Kubec 2008-06-30 13:48:08 UTC
Created attachment 63685 [details]
openide.dialogs diff
Comment 3 Milan Kubec 2008-06-30 13:48:57 UTC
Created attachment 63686 [details]
java.project diff
Comment 4 Milan Kubec 2008-06-30 13:54:03 UTC
I've attached diff files with the implementation, magic properties were made constants. Please review. Thanks.
Comment 5 Jesse Glick 2008-06-30 17:18:02 UTC
The suggestion in issue #54905 to add setter methods would be even better, as it would (1) enforce the correct value
types, (2) permit the impl to throw IAE for inappropriate arguments. This would be easy for wizard-wide properties, e.g.:

settings.putProperty("WizardPanel_errorMessage", NbBundle.getMessage(MakeSharableVisualPanel1.class,
"WARN_makeSharable.relativePath"));

=>

settings.setErrorMessage(NbBundle.getMessage(MakeSharableVisualPanel1.class, "WARN_makeSharable.relativePath"));

These could simply be new final methods on WizardDescriptor.

Properties set on wizard panels ("WizardPanel_contentData" etc.) are much uglier: (1) you have to cast to JComponent;
(2) you need to set redundant information on each panel (e.g. list of steps). Introducing constants improves the
situation somewhat, but perhaps we need a better API structure for this. Not obvious to me whether methods on WD would
work for these cases.
Comment 6 Tomas Mysik 2008-06-30 19:59:50 UTC
Milane, are you going to update all the WizardDescriptor usages?
Comment 7 Milan Kubec 2008-07-01 09:25:14 UTC
Jesse,
your comment is valid and using final methods would be the best solution, but from looking at the code and from talks with Jirka Rechtacek it seems to be 
out of scope of this issue. Using properties is not consistent (some of them affects only initial setup, some of them all panels, some of them needs to be set 
on each JComponent), so doing it right and then correctly replacing of all usages of put[Client]Property() would require more time than we have for 6.5M2 
IMO. Therefor I will proceed with the first step - making all those strings constants. 

> Milane, are you going to update all the WizardDescriptor usages?

I could, but I'm kind of afraid of HG, I don't want to spend two weeks of merging conflicts over number of HG clones ... at least I will try.

Comment 8 Tomas Mysik 2008-07-01 09:44:04 UTC
> Therefor I will proceed with the first step - making all those strings constants. 

Not sure whether it is a good idea to publish constants and then methods - constants would have to stay public, right?

>> Milane, are you going to update all the WizardDescriptor usages?
> I could, but I'm kind of afraid of HG, I don't want to spend two weeks of merging conflicts over number of HG
> clones ... at least I will try.

Thanks a lot, let me know if you have any problems, I could try to help you.
Comment 9 Milan Kubec 2008-07-01 10:19:02 UTC
>> Therefor I will proceed with the first step - making all those strings constants. 

> Not sure whether it is a good idea to publish constants and then methods - constants would have to stay public, right?

Well I suppose they will be deprecated as other public API being replaced by newer API.

Comment 10 Jesse Glick 2008-07-01 18:03:06 UTC
I agree with introducing documented constants for now. If we later introduced nicer methods, we would anyway need to
continue supporting the old properties (whether accessed via the new constants or not), so we could indeed just
deprecate the properties.

I don't understand the concern about replacing existing usages of the constants. Just do it and push to core-main
whenever you like. If and when there are merge conflicts with main, you will be notified, so just pull from main (at a
known good revision), merge (resolving the conflicts), and push back to core-main - as documented in
HgParallelTeamIntegrationBestPractices. Probably won't be any conflicts at all (most likely in import blocks), but if
there are, it should only take a few minutes to resolve.
Comment 11 Milan Kubec 2008-07-02 15:04:10 UTC
If there are no more objections I will push the patch tomorrow (together with fix of issue #137737). Thanks for review.
Comment 13 Jaroslav Tulach 2008-07-07 14:38:39 UTC
I have objection, but I guess I am too late here. You should use setter methods, not string constants. Constants 
improve almost nothing, with methods we could make the whole wizard experience much more friendlier...
Comment 14 Milan Kubec 2008-07-07 14:47:59 UTC
Jesse had the same concerns and I agree as well, but ... see "comments from mkubec Tue Jul 1 08:25:14 +0000 2008".

Comment 15 Jesse Glick 2008-07-08 13:17:19 UTC
You are missing @since on most of the new PROP_* constants; PROP_ERROR_MESSAGE has "3.39" for some reason.
Comment 16 Jesse Glick 2008-07-08 13:20:07 UTC
I am also putting comments in apisupport templates that you can use the new constants. (#d402c9f18abe) I am reluctant to
actually use the constants by default yet, because people may be developing against an older platform. (It is possible
to make the templates sensitive to the platform version, but this seems like a lot of work for a minor gain.)
Comment 17 Milan Kubec 2008-07-08 13:53:11 UTC
I actually didn't include @since on purpose, because those constants (as strings) were actually there from beginning, just they weren't public static final ... So 
I wasn't sure if @since is supposed to be there. PROP_ERROR_MESSAGE was added sometimes later, therefore @since 3.39.
Comment 18 Jesse Glick 2008-07-08 13:57:34 UTC
The @since should reflect when the constant was introduced into the API. There can be a separate note about how long the
constant's value has been interpreted.
Comment 19 Quality Engineering 2008-07-09 04:11:58 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #308 build
Changeset: http://hg.netbeans.org/main/rev/66cb4688e5d5
User: Milan Kubec <mkubec@netbeans.org>
Log: #137737, #138128: string properties changed to constants; new constants for warning and info messages introduced
Comment 20 Quality Engineering 2008-07-09 15:42:46 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #309 build
Changeset: http://hg.netbeans.org/main/rev/6051247ca234
User: Milan Kubec <mkubec@netbeans.org>
Log: #138128: missing @since tags for new constants added
Comment 21 Quality Engineering 2008-07-10 04:13:07 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #310 build
Changeset: http://hg.netbeans.org/main/rev/d402c9f18abe
User: Jesse Glick <jglick@netbeans.org>
Log: Noting the existence of WizardDescriptor.PROP_* constants. (cf. #138128)