In message <[EMAIL PROTECTED]>, steve cohen writes: >1) deprecate listFiles() methods that take a FileEntryParser parameter. >2) reimplement listFiles() to do what getFileList(null) now does. >3) reimplement listFiles(String pathname) to do what getFileList(null, >pathname) now does. >4) rename getFileList(String parserKey, String pathname) to listFiles(String >parserKey, String pathname). Even though autodetection will be the primary >use case, it already will not work for every system (see EnterpriseUnix) and >there must be some way around it. This is that way. >5) don't use properties to select the parser factory in our library. Instead, >add a property and a setter method. Initialize in the library to use the >default. Have I missed anything?
Yes, that's exactly what I was suggesting. It also occurred to me later that we may want to cache the system name in getSystemName (save it on the first call, return it on subsequent calls, clear it on disconnect), since it will be called more frequently now that you've added autodetection. >One problem remains. getFileList() throws a new exception: >ParserInitializationException. If we simply rename these methods to >listFiles() we will break lots of code. That was my primary reason for giving > >them a different name. One way around this would be to make >ParserInitializationException a RuntimeException. That makes sense, as the >error is not recoverable and is always due to a programming error - passing >in a key that the system cannot parse, specifying a class that has not been >made available. Would you agree? Another choice is to make it extend IOException, but that doesn't seem right. I guess my preference would be to catch the exception in listFiles() and rethrow an IOException that bubbles up the original exception. The problem with that is that standard support for wrapping exceptions is present only in J2SE 1.4. Also, what's the real difference between adding an FTPFileListException (or whatever name) derived from IOException and throwing that instead of just letting a ParserInitializationException derived from RuntimeException pass through. The reason I'm conflicted is that it doesn't seem to me that the caller of listFiles should be bothered to know about the internal implementation of the method, which the throwing of ParserInitializationException requires. If we make ParserInitializationException a RuntimeException we can always go with the catch and throw later. Still, I'm wondering if FTPFileEntryParserFactory should have a boolean isRecognized(String key) method or something similar to avoid the expense of a try/catch block (i.e., you ensure a key is supported before calling createFileEntryParser, and throw an FTPSystemNotSupportedException or something). I'm also wondering a good number of other things (like how to avoid new FooEntryParser() in createFileEntryParser on every call to listFiles), so I'll stop now and say that I support your suggestion to make ParserInitializationException a RuntimeException. It doesn't preclude additional tweaks we may think of before a final release. >in Effective Java lays this out clearly enough. If it was called >PreconditionViolationException or some such, it would be much clearer. As it >is, I've seen too many instance of exceptions being made RuntimeExceptions >simply because the programmer thought that propagating it up though the call >stack was too difficult. But I don't think that's the case here. This is >legitimately a RuntimeException). I agree, but it depends on how you anticipate the method being used. An argument can be made for createFileEntryParser to not throw an exception and instead return null. I wouldn't advocate that here because we're talking about a factory which creates new objects and not a mapper that pairs keys with pre-existing objects. >Agree here too, although the idea that FTPFileListParserImpl will no longer >implement FTPFileListParser is extremely odd. I suggest we might want rename >this class now to FTPFileEntryParserImpl. As it's abstract, I don't think >we'd be impacting anyone. That sounds like the right thing to do. So I guess we also deprecate FTPFileListParserImpl? daniel --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
