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: | Utility method for displaying the database connections in a combo box | ||
---|---|---|---|
Product: | db | Reporter: | Andrei Badea <abadea> |
Component: | Code | Assignee: | Andrei Badea <abadea> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | apireviews, lkotouc |
Priority: | P2 | Keywords: | API_REVIEW_FAST |
Version: | 5.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | ENHANCEMENT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 73678 | ||
Attachments: |
Proposed change
Javadoc Proposed change (updated) Javadoc (updated) |
Description
Andrei Badea
2006-03-17 16:26:12 UTC
Created attachment 29292 [details]
Proposed change
Created attachment 29293 [details]
Javadoc
There is missing @since tag in declaration of the connect method. Maybe the method should take ConnectionManager as argument. I would not say this if the ConnectionManager had only static methods, but in fact it has instances and thus it makes sense to select the one to work on. If you do not accept this comment then please at least improve javadoc to mention that the method operates on ConnectionManager.getDefault(). Re. name of the class - I do not like it much, but I agree that the method should be in separate class and not (for example) in ConnectionManager. Maybe "DatabaseUIs" would be my preferred choice. However there is a more important aspect that probably Jesse should comment on - he likes to separate APIs into "necessary" and "helper" ones - this UI method is clearly just a helper which everyone could write by themselves, so I expect Jesse to suggest to follow convention of projects and to create separate package - org.nb.api.db.support and put all such "helper" classes there. PS: Why Libor is author of all changes in the diff and Andrei is submitting them? Re. missing @since tag: I thought that it is not necessary to add @since tags to all methods of a new class and that it is enough to put a tag on the class itself. Am I wrong? Re. ConnectionManager parameter: I agree. Although ConnectionManager is a singleton now, this could save a deprecated method if in the future I decide it shouldn't be a singleton. Re. class and package name: I don't like the name either, but couldn't come up with anything better. DatabaseUIs and o.n.api.db.support seem too generic to me. The package name suggests it could contain more database-related APIs than those from the db module, but no other module than db would be able to add claseses to it, since we don't like shared packages. Same for the class. Therefore I would propose: org.netbeans.api.db.explorer.support.DatabaseExplorerUIs Re. other submitter than author: I should have written this class at Libor's request, but since Libor had already created a similar one for data sources, I asked him to write this one too. He didn't have time to ask for a review because he was busy preparing his own review of the data sources API. I will attach a new diff when a consensus is reached over the naming. All agreed and sorry for the confusion with @since tag. Created attachment 29324 [details]
Proposed change (updated)
Created attachment 29325 [details]
Javadoc (updated)
If there are no more comments, the change will be commited to the release55 branch next week. Thank you for the review. Changes were commited into release55 branch. |