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.

Bug 81527 - Overcomplicated code for creating directory for a new project
Summary: Overcomplicated code for creating directory for a new project
Status: CLOSED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Filesystems (show other bugs)
Version: 5.x
Hardware: All All
: P3 blocker (vote)
Assignee: rmatous
URL:
Keywords: API_REVIEW_FAST
Depends on:
Blocks: 84813 82815
  Show dependency tree
 
Reported: 2006-07-31 19:45 UTC by Martin Krauskopf
Modified: 2012-04-05 14:49 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
FileUtil.createData, createFolder (7.33 KB, patch)
2006-08-29 10:46 UTC, rmatous
Details | Diff
improved tests + javadoc changes (8.80 KB, patch)
2006-08-29 18:45 UTC, rmatous
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Krauskopf 2006-07-31 19:45:05 UTC
Three methods (around 40 lines of code) are needed(?) just for creating one
directory which seems to be wrong and was probably needed in the past due to...
I don't know what. Radku (FS) could you help with evaluating this and possibly
fixing it. Concerned methods are:

- NbModuleProjectGenerator.[createProjectDir,refreshFolder,refreshFileSystem]

It is more or less copy-pasted from j2seproject and probably also in other
project types which causing hardly maintainable code.... just playing with
another project-type and end-up with copy-pasting those three methods as well in
uncertainty. Thanks.
Comment 1 Petr Jiricka 2006-08-01 18:48:22 UTC
Yes, this is interesting. I was thinking that it may be useful to have methods:
FileObject FileUtil.createData(URL u)
FileObject FileUtil.createFolder(URL u)

which would take any URL accepted by URLMapper. Looks like it would help for
this particular issue.
Comment 2 Martin Krauskopf 2006-08-16 08:05:30 UTC
It's a TASK.
Comment 3 Jesse Glick 2006-08-16 18:29:36 UTC
Sure it's a TASK, but P2 still seems too high for code which AFAIK is working fine.
Comment 4 Martin Krauskopf 2006-08-17 07:46:12 UTC
The problem is that there were a P1 (issue 81039) in this code which was
discovered in J2SE project. And since nobody is 100% sure about why this code
for directory creation is so "strange" and if it is still really needed,
everybody rather copy-pasted it into a new project-type. Like all those in Java
EE (web, car, ear, ejb, archive, ....) and few others in core, e.g. apisupport.
So we end up with serveral yet not filed P1s (yup, that bug is IMHO P3).
Comment 5 Jesse Glick 2006-08-17 17:08:48 UTC
OK, if we don't know whether or why some code works then that is a bug.

As an aside, I find the Filesystems API more trouble than it's worth for this
kind of thing. Would be easier to just use File.exists(), File.mkdirs(), etc.,
which are well-documented, well-tested, and reliable. The unfortunately design
decision was to have a FileObject not be a path (like File) but a live object
that must refer to an existing file, even though we cannot in fact ensure that
this is true since we have no reliable listening capability. The only thing the
FS API buys us here is change notification, which could probably be done just as
easily in another way.
Comment 6 rmatous 2006-08-29 10:46:47 UTC
Created attachment 33353 [details]
FileUtil.createData, createFolder
Comment 7 Tomas Zezula 2006-08-29 13:45:01 UTC
The createFolder is very helpful for the ProjectGenerators. Each project
generator contains code similar to this. These helper methods are fine for
me.
Comment 8 Jesse Glick 2006-08-29 15:41:28 UTC
Javadoc for params is incomplete and has a duplicated tag, pls. recheck, and
also add @since.


The test is quite inadequate. "doesn't work for LocalFileSystem (just on
MasterFS) - probably not needed" - well is it needed or not? If it is, then the
test should show why; if it isn't then delete it. "if refresh needed because of
external changes" - this is not tested at all.

The test does not even check that the return value is correct, only that it is
not null. It does not really try creating a data file (since a folder with the
same name already exists) - in fact the Javadoc fails to specify what the tested
createData should actually do.

There are other things the test could probably check as well: what happens when
you pass a nonexistent drive letter on Windows; what happens when some
intermediate component exists but is read-only.
Comment 9 rmatous 2006-08-29 18:45:36 UTC
Created attachment 33368 [details]
improved tests + javadoc changes
Comment 10 Jesse Glick 2006-08-29 20:17:11 UTC
Test looks better.


testCreateFolderOrDataFileWithNotExistingRoot() will do something weird on Unix
I guess. You probably meant to write

if (!Utilities.isWindows()) {
    return;
}
File wDir = null;
for (char d = 'A'; d < 'Z'; d++) {
    File attempt = new File(String.valueOf(d)+":/");
    if (!attempt.exists()) {
        wDir = attempt;
        break;
    }
}
if (wDir == null) {
    return;
}
Comment 11 rmatous 2006-08-30 14:08:10 UTC
Maybe !Utilities.isWindows() but if wDir doesn't exist and wDir is root because
wDir.getParentFile() == null then no subfile can be created and thus IOException
will be thrown on all platforms. Then I don't find it so weird on Unix.
Comment 12 rmatous 2006-09-05 10:54:33 UTC
I'm going to integrate latest patch into trunk.
Comment 13 rmatous 2006-09-07 09:45:01 UTC
integrated into trunk:
/cvs/openide/fs/src/org/openide/filesystems/FileUtil.java,v  <--  FileUtil.java
new revision: 1.26; previous revision: 1.25

/cvs/openide/masterfs/test/unit/src/org/netbeans/modules/masterfs/MasterFileObjectTestHid.java,v
 new revision: 1.34; previous revision: 1.33

/cvs/openide/fs/apichanges.xml,v  <--  apichanges.xml
new revision: 1.11; previous revision: 1.10
Comment 14 rmatous 2006-09-07 09:45:35 UTC
Fixed.
Comment 15 Martin Krauskopf 2006-09-07 13:32:17 UTC
Great. -113 lines of code just in APISupport ;)

v/c

NbModuleProjectGenerator.java; 1.46 -> 1.47;
suite/SuiteProjectGenerator.java; 1.10 -> 1.11;