7

I'm creating a command-line utility that will delete sub-directories/files. If a file is in use, the System.IO.IOException is thrown. I use a try-catch block within my for loop.

Question:

1.Is it bad practice to have a try-catch within a for loop?

2.If Yes, what is a better alternative?

My Code:

  System.IO.DirectoryInfo di = new DirectoryInfo(path);

    foreach (FileInfo file in di.GetFiles())
    {
         try
         {
             file.Delete(); 
         }
         catch(System.IO.IOException)
         {
           Console.WriteLine("Please Close the following File {0}", file.Name);
         }

    }
Just Do It
  • 461
  • 1
  • 7
  • 18
Emma Geller-Green
  • 297
  • 1
  • 4
  • 9
  • The message assumes that the failure is because the file is opened. What if the file cannot be deleted for some other reason? – Eric Lippert Jan 08 '16 at 19:41
  • @EricLippert - I was thinking exactly that, but, I'm unsure of how to proceed. How would you go about handling if a file cannot be deleted for another reason? – Emma Geller-Green Jan 08 '16 at 20:42
  • Well, first, decide if anyone cares. If no one cares, don't stress about it. If someone cares, next thing to do is to decide how to tell them. Console output is maybe a good way, maybe not. Hard to say without understanding the rest of the program. Next thing to do is to decide what to tell them. Tell them the truth: "file blah.txt could not be deleted". Consider giving the message from the exception to add more context. But the truth that you know here is that the file could not be deleted, so start with that. – Eric Lippert Jan 08 '16 at 20:44
  • I'd guess since it's console output any user using this will understand that "could not be deleted" probably means they need to close the file. Otherwise could add suggestion "if file is open, close it" or something. – Matthew Jan 11 '16 at 14:57

2 Answers2

15

No this can be quite useful. For example: If you didn't want to completely stop the loop if an Exception was thrown, or if there was extra code that shouldn't be ran for the current iteration due to the Exception you could do something like the following.

System.IO.DirectoryInfo di = new DirectoryInfo(path);

foreach (FileInfo file in di.GetFiles())
{
     try
     {
         file.Delete(); 
     }
     catch(System.IO.IOException)
     {
       Console.WriteLine("Please Close the following File {0}", file.Name);
       continue;
     }
     //
     // Other Code 
     //
}

This way you can log the error to review later but still process the rest of what you were trying to process.

mac
  • 485
  • 1
  • 6
  • 29
  • you could also use this catch to do something like delete the file on reboot. http://stackoverflow.com/questions/6077869/movefile-function-in-c-sharp-delete-file-after-reboot – Matthew Whited Jan 08 '16 at 18:06
  • @Mac - Many thanks for the code, and for answering my question. – Emma Geller-Green Jan 08 '16 at 18:06
  • 2
    The `continue` here is superfluous; you're already at the end of the body of the loop, so the `continue` isn't skipping over anything. – Servy Jan 08 '16 at 18:32
  • 1
    @Servy - You're right. I should have elaborated with more example code. If there is any extra code related to the processing of the file you don't want to run it. Instead you want to skip to the next file. – mac Jan 08 '16 at 18:39
0

No its not a bad practice to skip the exception deliberately. But looking at your use case

I'm creating a command-line utility that will delete sub-directories/files. If a file is in use, the System.IO.IOException is thrown.

I would suggest you to delete the directory rather than looping on each file in the subfolder. https://learn.microsoft.com/en-us/dotnet/api/system.io.directory.delete?view=net-5.0

Kundan Kumar
  • 1