At 02:28 AM 2/7/2003, Stefan Bodewig wrote:

I am aware that this could lead to a VFS layer and has a lot of
potential for future expansions, but for now it has been kept as a
bare minimum.

One thing that I might want to see some stricter contract enforced is
the name.  getName() will currently return names using the platform's
file separator as separator for Files and forward slashes for
ZipEntryies.

I'd prefer to make that always use something predictable, and the
platform's separator may be the better choice.

Depending on what you mean by "platform", I think this would would not be the best choice since it makes the VFS layer you talked about more complicated to eventually do. Would you want URLs, for example, to have forward slashes converted to a platform's separators?


If consistency is needed, my preference would be to define a canonical "Resource" path separator that works on the majority of potential filesystems (aka '/'). This may add a bit more complication up-front, but it has the virtue that it will still work in the future we envisage for ourselves. The downside is that on some platforms it will involve a lot more conversion back and forth.

> (3) New interface ResourceScanner

I'm not convinced that this is really needed (at this point).

I have to agree. I'm not sure I see much benefit to an interface like this until we go for a fully abstracted file system.



> (4) New utility class SourceSelector.
>
> chosing some better names on the way.

At least I hope so. 8-)

If the comment at the top of the file is a true indication of what it is for, then how about "ResourceProcessingUtils", or perhaps more simply "ResourceUtils".



I have an issue with the implementation, not with the API.

ZipScanner now wants a Task instance to log to.  This should probably
be a ProjectComponent so that ZipFileSet can point to itself in
getDirectoryScanner.  Or maybe it should get the Project instance
directly?

Is it necessary that Scanners call the logging API? Couldn't the layer around them do that, relying on the information provided through toString() and BuildExceptions?



Furthermore, errors get just logged, I'd prefer to really bail out and
throw a BuildException here.  The problem is that these are going to happen in
getIncluded*() methods, that are not declared to throw anything (I
know BuildException is not a checked exception, but it doesn't look
right to throw them without telling the user).

As someone who hates checked exceptions, I'm probably the wrong person to talk to this point. I'll just point out that BuildExceptions already percolate throughout the code without necessarily being declared.





Reply via email to