2009/3/9 Marnie McCormack <[email protected]>: > Hi Martin, > > In general terms I would think this isn't a great thing - it could, for > example, indicate that the file the code is trying to delete never > existed/is incorrect so the calling code needs to care about what happened > if the delete fails for that reason. > > I'd suggest that this method should throw the exceptions rather than return > a boolean, so the calling code can handle as appropriate.
I agree, was trying to fix a problem the wrong way. I have reverted the changes and FU.delete() will now behave as you would expect with java.io.File. I think the null values that I was seeing being passed in were an artefact of the other failure I was seeing. Martin > Regards, > Marnie > > On Fri, Mar 6, 2009 at 3:46 PM, <[email protected]> wrote: > >> Author: ritchiem >> Date: Fri Mar 6 15:46:14 2009 >> New Revision: 750946 >> >> URL: http://svn.apache.org/viewvc?rev=750946&view=rev >> Log: >> FileUtils : Was not correctly handling the case where a File object became >> null, it would previously have thrown a NPE which was erroneously caught >> this and declared the delete to have failed. If there is nothing to delete >> (signified by the Null File object) then the delete should pass. >> >> Modified: >> >> >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java >> >> Modified: >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java >> URL: >> http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java?rev=750946&r1=750945&r2=750946&view=diff >> >> ============================================================================== >> --- >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java >> (original) >> +++ >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/util/FileUtils.java >> Fri Mar 6 15:46:14 2009 >> @@ -246,18 +246,19 @@ >> { >> boolean success = true; >> >> + // If we have nothing to delete then it must be ok to say it was >> deleted. >> + if (file == null) >> + { >> + return true; >> + } >> + >> if (file.isDirectory()) >> { >> if (recursive) >> { >> - try{ >> - for (File subFile : file.listFiles()) >> - { >> - success = delete(subFile, true) & success ; >> - } >> - }catch (NullPointerException npe) >> + for (File subFile : file.listFiles()) >> { >> - success = false; >> + success = delete(subFile, true) & success ; >> } >> >> return success && file.delete(); >> >> >> >> --------------------------------------------------------------------- >> Apache Qpid - AMQP Messaging Implementation >> Project: http://qpid.apache.org >> Use/Interact: mailto:[email protected] >> >> > -- Martin Ritchie --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:[email protected]
