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: | WebModule API review | ||
---|---|---|---|
Product: | javaee | Reporter: | Pavel Buzek <pbuzek> |
Component: | Code | Assignee: | Pavel Buzek <pbuzek> |
Status: | CLOSED FIXED | ||
Severity: | blocker | Keywords: | API_REVIEW_FAST |
Priority: | P2 | ||
Version: | 4.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Attachments: | Javadoc documentation and architecture document |
Description
Pavel Buzek
2004-06-15 02:06:37 UTC
Created attachment 15691 [details]
Javadoc documentation and architecture document
Please replace or enhance "Use classpath API to obtain classpath for the document base." with a code snippet. Do not expose Node.Cookie in API. The only place it is currently still necessary is DataObject, but Node already supports Lookup. I know that the usage is connected to data objects, but maybe you could replace this with the query pattern used in projects. According to famous Effective Java book, the WebProjectConstants is poor pattern. SPI looks perfect to me. Please make sure the api appears on http://www.netbeans.org/download/dev/javadoc/ I copied the pattern from JavaProjectConstants that went through this kind of review and is unchanged. If you have a particular suggestion please explain it to me. Actually JavaProjectConstants was added after the main buildsys inception review. I will change it to be a class with a private constructor. This set of comments is related to the API. Interface URLCookie: Since the getURL() method doesn't return a full URL, it should not be named getURL. I would recommend using a name that parallels the names used in the java.net.URL. Based on your description, getFile() is probably the closest match. Interface WebProjectConstants: This should be a class NOT an interface. If JavaProjectsConstants has this bug, you should not follow it. Please read Item 17 in Effective Java. It is on Page 89 of the copy that I have. Class WebModule: It seems like this object should implement javax.enterprise.deploy.model.DeployableObject. Why doesn't it? If you implement equals(Object), you should probably also implement hashCode(). Item 8 in Effective Java. I think the api is missing a couple getters: getMetaInf() - in case some module will need to put data in there. getClasses(), getLib(), getTld() - in case we want to move these from underneath the FileObject that is the return value of getWebInf(). This would allow us to change the web module projects physical layout without breaking its clients.... URLCookie - I will change this to query pattern, implement it in
project and store the settings in private.propeties instead of
nbattrs. I agree getURL() is not a good name. I am leaning towards
something even less general then getFile() - what about
getRequestPath()? Any other suggestions?
> It seems like this object should implement
> javax.enterprise.deploy.model.DeployableObject. Why doesn't it?
1. Why should it? You can ask for both interfaces separately.
2. sometime it is not needed - freeform project will use Ant tasks for
deployment so it does not need j2eeserver api implemented on it
I would prefer not to mix these 2 things.
This reminds me that when we get to j2eeserver review we may want to
change the J2eeModuleProvider to query pattern, but that's a separate
issue.
getLib, getClasses,getTags - what are they needed for? Of course
clients should not use getFileObject ("WEB-INF/classes") but they are
not used in existing code, AFAIK. Provide use cases if you want them
added. (I assume this is in build area, right?)
the rest I will fix (WebModule.hasCode, WebProjectConstants being
class, doc about using classpath)
What is this? Forgotten review? Was it integrated? Is the javadoc published? At least a fake of tests written? Please update the status here. Oops. Looks like you are right. I fixed the poor pattern in WebProjectConstants and added code sample for classpath in WebModule. I have not fixed the URLCookie. It can be fixed in 4.1 (this is a friend API) or in 4.0 if you insist on it. Obviously I'd prefer to have a P2 for 4.1 and close this issue. There are actually some fake tests :-) Are they too much fake? The javadoc is not published but that can be fixed very easily if you think it should be published (even when I have not fixed 1 API issue). cvs diff -u build.properties (in directory E:\nb_all\nbbuild\) Index: build.properties =================================================================== RCS file: /cvs/nbbuild/build.properties,v retrieving revision 1.219 diff -u -r1.219 build.properties --- build.properties 12 Nov 2004 22:09:47 -0000 1.219 +++ build.properties 22 Nov 2004 19:18:47 -0000 @@ -364,6 +364,7 @@ debuggerjpda/api, \ ant/project, \ j2eeserver, \ + web/webapi, \ java/api, \ java/javacore, \ java/javamodel, \ Let me know what I should do. Publish javadoc for 4.1 (release40 contains stable apis only anyway). Otherwise I am fine with the rest for 4.1 (I think). the right term for request parameters is defined in j2ee/platform/javahelp/org/netbeans/modules/j2ee/platform/docs/web/glossary/URL_query_string.html - URLQueryString URLCookie removed. Closing as fixed. v |