Bug 232242 - refactor profile related methods from Util class to Profile class
refactor profile related methods from Util class to Profile class
Status: RESOLVED FIXED
Product: javaee
Classification: Unclassified
Component: Code
7.3.1
All All
: P2 (vote)
: 7.4
Assigned To: David Konecny
issues@javaee
: API_REVIEW_FAST
Depends on:
Blocks: 230820 231806
  Show dependency treegraph
 
Reported: 2013-07-04 04:02 UTC by David Konecny
Modified: 2013-07-11 04:10 UTC (History)
3 users (show)

See Also:
Issue Type: TASK
:


Attachments
patch (14.44 KB, patch)
2013-07-04 04:05 UTC, David Konecny
Details | Diff
new patch (116.21 KB, patch)
2013-07-04 23:48 UTC, David Konecny
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Konecny 2013-07-04 04:02:17 UTC
org.netbeans.modules.j2ee.common.Util contains several Profile related methods which should be moved to Profile class.
Comment 1 David Konecny 2013-07-04 04:05:14 UTC
Created attachment 136690 [details]
patch
Comment 2 David Konecny 2013-07-04 04:11:48 UTC
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.
Comment 3 Martin Fousek 2013-07-04 12:09:08 UTC
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.
Comment 4 Martin Fousek 2013-07-04 12:24:35 UTC
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)
Comment 5 David Konecny 2013-07-04 23:45:59 UTC
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.
Comment 6 David Konecny 2013-07-04 23:48:23 UTC
Created attachment 136732 [details]
new patch
Comment 7 David Konecny 2013-07-08 22:24:03 UTC
Thanks for the review. Integrated as web-main#85d243030c08
Comment 8 Martin Janicek 2013-07-09 10:53:23 UTC
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..
Comment 9 David Konecny 2013-07-10 00:20:40 UTC
MJ01, MJ02 - fixed in 2d6ac0d64fe4

MJ03 - can be added when needed by somebody how needs it :-)
Comment 10 Quality Engineering 2013-07-10 04:42:03 UTC
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
Comment 11 Quality Engineering 2013-07-11 04:10:30 UTC
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


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