Refactor this nested IF function that is wrapped in a try/catch

Question

Hi,

I have a messy function that needs refactoring, it has too many nested IF's and it makes me nervous just to look at it!

Please ignore what the functions are doing, I'm more concerned with the structure/flow and how it can be refactored so it has less nested IF statements

The basic flow is as follows:

public static void HandleUploadedFile(string filename)
{

      try
      {
     if(IsValidFileFormat(filename)
     {

      int folderID = GetFolderIDFromFilename(filename);

      if(folderID > 0)
      {

       if(HasNoViruses(filename)
       {

        if(VerifyFileSize(filename)
        {

         // file is OK
         MoveToSafeFolder(filename);

        }
        else
        {
         DeleteFile(filename);
        }


       }
       else
       {
        DeleteFile(filename);
       }


      }
      else
      {
       DeleteFile(filename);
      }



     }
     else
     {
      DeleteFile(filename);
     }
      }
      catch (Exception ex)
      {

      }
      finally
      {
     // do some things
      }


}

Answer 1

I would be tempted to go as far as:

    private static bool CanMoveToSafeFolder(string filename)
    {
        return IsValidFileFormat(filename)
            && GetFolderIDFromFilename(filename) > 0
            && HasNoViruses(filename)
            && VerifyFileSize(filename);
    }

    public static void HandleUploadedFile(string filename)
    {

        try
        {

            if (CanMoveToSafeFolder(filename))
            {
                // file is OK
                MoveToSafeFolder(filename);
            }
            else
            {
                DeleteFile(filename);
            }
        }
        catch (Exception ex)
        {

        }
        finally
        {
            // do some things
        }

    }

Answer 2

The following is equivalent to your original code.

public static void HandleUploadedFile(string filename)
{
    try
    {
        if( IsValidFileFormat(filename) && 
            (GetFolderIDFromFilename(filename) > 0) && 
            HasNoViruses(filename) &&
            VerifyFileSize(filename) )
        {
              MoveToSafeFolder(filename);
        }
        else
        {
              DeleteFile(filename);
        }
    }
    catch (Exception ex)
    {
        // do some things
    }
    finally
    {
        // do some cleanup
    }
}

Answer 3

This honors the original conditions under which the file is deleted and is cleaner and more compact. My first answer set folderId, but I then realized it isn't used.

public static void HandleUploadedFile(string filename) {
    try {
        if(IsValidFileFormat(filename) 
        && GetFolderIDFromFilename(filename)>0 
        && HasNoViruses(filename) 
        && VerifyFileSize(filename)) {
            MoveToSafeFolder(filename);                                 // file is OK
            }
        else {
            DeleteFile(filename);
            }
        }
    catch (Exception ex) {
        // HANDLE THE EXCEPTION; AT LEAST LOG IT!
        }
    finally {
        // do some things
        }
    }