Alan Bateman wrote:
Jean-Christophe Collet wrote:
Hello,
I have posted an entry in my blog about the current status of the FTP
client API.
It contains a quick description of the project as well as a link to
the current draft of the API. So if you're interested in that topic
go take a look at http://blogs.sun.com/jcc/
As mentioned in the post, feedback is very strongly encouraged.
I'm interested in reviewing this. From an initial glance this is ftp
and ftps - are you also thinking about sftp? Some initial comments
from a first pass:
It would be better to use a static factory method rather than public
constructors. For one thing, there are a slew of ftp clients about and
you'll likely get requests for a provider interface so that
alternative implementations can be used.
Good point. Current design inherited a lot from the existing
sun.net.ftp.FtpClient code (for obvious compatibility reasons), but
that's definitely one thing we can improve.
The client has state (not connected, connected, logged, etc.) and you
might want to introduce a few specific exceptions for this. For
example, what does connect throw if you are already connected, what do
the other methods throw when you aren't connected, etc.
In current draft there are quite a few methods that return a boolean
and throw IOException. It's not clear to me what what failure means -
do these methods return false or do they throw an I/O exception? How
is this used with the getLastReplyCode method or should the exception
have a method to return the reply code?
The idea is to differentiate errors reported by the FTP server (aka non
fatal errors), like 'bad login' or file not found, from the serious
networking errors (connection lost, no route to host etc...).
In the first case, the current connection can still be used for other
attempts, while in the second case it's more likely too serious to recover.
The listFiles returns a List<FtpFile> and isn't going to scale with
large directories (you'll have to fetch the entire directory from the
server for this method to return). It would be better to return
something that implements Iterable<FtpFile> and Closeable.
This seems a common concern. I'll update the API accordingly.
I'm curious about FtpFile. My memory of the ftp protocol is hazy but I
thought that commands such as LIST or NLIST didn't specify the format
of the file attributes. There was a IEFT WG working on defining these
but I don't see this in the references. Do you have a pointer to that?
They don't. That's why the API provides a pluggable 'directory parser'.
By default we provide a parser that is able to 'grok' most of what is
out there, but in case it's needed, the application can provide its own.
I see that the getModificationTime returns a Date. Care to revise your
statement sir :-)
Shows how long this API has been in the working....
I will change that.
You've got a number of setter methods that return void. It might be
nicer for them to return this so as to facilitate invocation chaining.
Good point.
Should the members of the TransferMode enum be in uppercase?
Probably.
Same for TransferType (and I probably should remove EBCDIC since it's
not really supported).
Do you really need the ability to set/get the connect timeout and
allow it be overridden by the connect method?
I guess this is a bit redundant. Will clarify.
That's it for a first pass.
Thanks,
