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.
As part of http://wiki.netbeans.org/J2eeCommonRefactoring the Util class was identified as a collection of random utilities and needs to be clean up little bit.
Created attachment 136299 [details] patch This is a first iteration with several changes in Util class: * updateDirsAttributeInCPSItem, initTwoColumnTableVisualProperties, backupBuildImplFile moved to new project.ProjectUtils class * changeLabelInComponent, hideLabelAndLabelFor, getAllComponents, findLabel are used only from single module so I moved the methods to that module: websvc.core/src/org/netbeans/modules/websvc/core/dev/wizard/Utils.java * getFullClasspath, getJ2eeSpecificationLabel were deleted without replacements (they were not used) * isSourceLevel14orLower - obsolete and was deleted including all code calling it * isSourceLevel16orHigher - moved to websvc.core/src/org/netbeans/modules/websvc/core/dev/wizard/Utils.java (the only client) * getSourceLevel deleted - calls replaced with SourceLevelQuery.getSourceLevel * all containsClass* methods were moved into new ClasspathUtil class * fileToUrl was deleted and calls were replaced with FileUtil.urlForArchiveOrDir
Petr, Milan, MartinF, could you look at the attached patch and let me know if you find the changes OK. It's mostly shuffling code around. Any suggestions for improvements are welcome. This patch does not resolve this issue completely - it is just a first batch of work. But the patch is already big enough. I'm not sure about ProjectUtils and ClasspathUtil classes but at least the names represent what methods can be expected inside. ClasspathUtil could be moved into j2ee.core.utilities into classpath package maybe? What do you think guys? Thanks.
(In reply to comment #2) That's really good piece of work. > Any suggestions for improvements are welcome. MF01: Many FileUtil.urlForArchiveOrDir() calls are not check for null value, especially before adding result into a collection. The previous code mostly caught the MalformedURLException with INFO or WARNING message. This could lead newly to pack of NPEs. MF02: SourceLevelQuery.getSourceLevel() can return null, please change the call to something like '"1.5".equals(SourceLevelQuery.getSourceLevel(...))'. MF03: isSourceLevel14orLower: The second option would be place this method within the websvc.core module which is the only client. Do we know which Java source levels are supported? If all I would move it there. But I don't have strong opinion in this. > I'm not sure about ProjectUtils and ClasspathUtil classes but at least the > names represent what methods can be expected inside. ClasspathUtil could be > moved into j2ee.core.utilities into classpath package maybe? The naming as well as the move looks logically to me. Sorry no better idea.
Thanks for the refactoring. I like you've created Utils in websvc.core module, and of course moving stuff from org.netbeans.modules.j2ee.common.Util, and removing some duplicities. Vote for MF02.
Thanks for comments Martin. Re. MF01 - I had done that in few case which looked troublesome. In all other cases the file is tested for existence before FileUtil.urlForArchiveOrDir() is called. Which should be enough protection for NPE. If it would be one or two cases I would fix it but it is too many of them. I will have a one more look at it tomorrow. Re. MF02 - I'm aware of lack of NPE testing but original code had the same problem. I will have a look at it tomorrow how many cases needs to be fixed. Re. MF03 - not worth it. "JDK 1.4 or older" is just very old code. EE5 is last EE spec supported by NB. I will do some work on it hopefully tomorrow and see if I can improve it. I will try to push this changeset tomorrow too.
(In reply to comment #5) > Re. MF03 - not worth it. "JDK 1.4 or older" is just very old code. EE5 is last > EE spec supported by NB. That's right, I agree.
The change was pushed - web-main#51f4e6fabd8c. I fixed both MF01 and MF02. I'm leaving the issue open as there are few other methods to sort out.
(In reply to comment #7) > The change was pushed - web-main#51f4e6fabd8c. I fixed both MF01 and MF02. Looks great, thanks!
Integrated into 'main-silver', will be available in build *201307011244* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/51f4e6fabd8c User: David Konecny <dkonecny@netbeans.org> Log: #231806 - clean-up org.netbeans.modules.j2ee.common.Util
Few more refactorings I'm just finishing (web-main#4960f0cdf5c6): * added ProjectConstants * added UIUtil * moved remaining project related methods to ProjectUtil Util now have only few isxxxProfile methods. MartinF, have you wanted to do something with those methods? Or am I confusing it with other code? If so please re-assign this issue to yourself and finish it. If not then I'm thinking of: * move isJavaEE5orHigher(Project project) to ProjectUtils * move remaining methods to Profile class itself or somewhere next to it?
(In reply to comment #10) > Util now have only few isxxxProfile methods. MartinF, have you wanted to do > something with those methods? Or am I confusing it with other code? No, it should be related to something a little bit different and I suppose that it should still use hugely these Util methods. > * move remaining methods to Profile class itself or somewhere next to it? This makes good sense to me since it will place Profile and it's utility methods at one place. The disadvantage is placing into the public API, but I believe that this is useful to anybody who works with Profile information.
Integrated into 'main-silver', will be available in build *201307022300* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/4960f0cdf5c6 User: David Konecny <dkonecny@netbeans.org> Log: #231806 - clean-up org.netbeans.modules.j2ee.common.Util
Created attachment 136691 [details] remaining changes to eliminate Util class completely API review was filed separately as issue 232242.
[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.
(In reply to comment #13) > Created attachment 136691 [details] > remaining changes to eliminate Util class completely Looks great! I would just increase spec. versions of modules and their dependencies. Thanks a lot. > API review was filed separately as issue 232242. I'll comment there.
This has been resolved now and Util class does not exist anymore (web-main#85d243030c08).