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.
| Summary: | refactor profile related methods from Util class to Profile class | ||
|---|---|---|---|
| Product: | javaee | Reporter: | David Konecny <dkonecny> |
| Component: | Code | Assignee: | David Konecny <dkonecny> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | marfous, mjanicek, phejl |
| Priority: | P2 | Keywords: | API_REVIEW_FAST |
| Version: | 7.3.1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | TASK | Exception Reporter: | |
| Bug Depends on: | |||
| Bug Blocks: | 230820, 231806 | ||
| Attachments: |
patch
new patch |
||
|
Description
David Konecny
2013-07-04 04:02:17 UTC
Created attachment 136690 [details]
patch
Please review. Attached patch eliminates j2ee.common/src/org/netbeans/modules/j2ee/common/Util.java (Friend API) and moves most of the methods to j2ee.core/src/org/netbeans/api/j2ee/core/Profile.java (Public API). The Util.isJavaEE5orHigher(Project) method will be moved to j2ee/common/project/ProjectUtil.java separately. Thanks. I suppose that phejl's comments should be placed here: [PH01] It seems to be pointless to have Profile.isSomething(Profile). Those methods should not be static. The calls should look like profile.isSomething(). [PH02] Not really a part of the API but I think now is the right time to put some basic tests for those methods. Thanks. MF01: Please consider merge of all public methods "isAtLeastJavaEE<XXX>" into one. I think that the original reason for specification and naming of these methods was fear from confusion and unreadability - "Util.isProfileAtLeast(Profile p1, Profile p2)". Now with the advantage of the Profile's instance method it makes pretty good sense to create isAtLeast(). The usage looks straightforward and generic for all added profiles in the future: myprofile.isAtLeast(Profile.JAVA_EE_7_WEB) PH01 - good point. I was just moving existing methods without trying to change them but you are right. PH02 - oops my fault. There were tests in UtilTest class which I forgot to move to ProfileTest MF01 - again, I agree. New patch resolves all above suggestions. And updates all existing references to old methods. The spec and dependency updates will be done before commit. Created attachment 136732 [details]
new patch
Thanks for the review. Integrated as web-main#85d243030c08 I know the review is closed already but if possible please consider following comments: MJ01: Please consider JavaDoc improvement for isAtLeast (something like second paragraph from isVersionEqualOrHigher should be present as the comparison rules might not be obvious) MJ02: @NonNull should be added for Profile parameter MJ03: Maybe we should think about contrary method isAtMost or something like that (there are cases when we are checking such conditions), but it can be done separately in the future.. MJ01, MJ02 - fixed in 2d6ac0d64fe4 MJ03 - can be added when needed by somebody how needs it :-) Integrated into 'main-silver', will be available in build *201307100049* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/85d243030c08 User: David Konecny <dkonecny@netbeans.org> Log: #232242 - refactor profile related methods from Util class to Profile class Integrated into 'main-silver', will be available in build *201307102300* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/2d6ac0d64fe4 User: David Konecny <dkonecny@netbeans.org> Log: #232242 - refactor profile related methods from Util class to Profile class |