2009/4/6 David Latorre (JIRA) <[email protected]>:
>  Are you subscribed to the developers mailing list? So we can move the 
> discussion there.

I'm pretty sure Sai is, so let's move.

>  I hadn't thought much about these changes myself but your work looks pretty 
> good.  The only thing I don't quite get is why we would use 
> FTPFile.getPhysicalFile ... Since it returns an Object, the coder developing 
> the FTPLet should know which 'PhysicalFile' object (s)he is using. This would 
> mean in most situations that they know which FTPFile implementation they're 
> using and hence, they could use casting to their appropriate type. And I 
> guess it's possible to have an implementation of FTPFile that doesn't use a 
> "PhysicalFile" object ... So what's your use case for this addition? I'm so 
> tired i can't clearly think...

I agree with David that the patch looks very good (I will review it a
bit more in detail later). I have previously argued against something
like getPhysicalFile, but I'm more and more leaning over to it being a
good idea. The reason for this is that we would encourage developers
to provide a standard way of getting the underlying object. But, the
javadocs should very clearly state that the method might return null,
for example if there is no reasonable object to return.

> Another problem we have is that it is becoming quite difficult to develop 
> pluggable components (be it an UserManager ,a Command or a ftplet) with only 
> the API documentation. How would a user know which types of FTPReply he 
> should use if overwriting a command? - I don't have a solution for this at 
> all. It's just something we could think about.

That is a very interesting comment. How about we attempt to provide a
reasonable simple factory? It would also fit well with the OSGi
support.

> Agree we should add a bug report for getUniqueFile(), using createTempFile 
> would be simpler but  it is probably better to generate an unique filename 
> and check for permission before creating the actual file  (so all these 
> should be under a static lock)  ; although of course checking only parent 
> directory permissions we are good to go right now.

Please open a JIRA and let's fix this in both trunk and the 1.0.x branch.

/niklas

Reply via email to