Refactor nested IF statement for clarity [closed]

Question

I want to refactor this mumbo jumbo of a method to make it more readible, it has way to many nested IF's for my liking.

How would you refactor this?

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);
            LogError("file size invalid");
          }
        }
        else
        {
          DeleteFile(filename);
          LogError("failed virus test");
        }
      }
      else
      {
        DeleteFile(filename);
        LogError("invalid folder ID");
      }
    }
    else
    {
      DeleteFile(filename);
      LogError("invalid file format");
    }
  }
  catch (Exception ex)
  {
    LogError("unknown error", ex.Message);
  }
  finally
  {
    // do some things
  }
}

Answer 1

I would reverse the conditions in the test to if bad then deleteAndLog as the example below. This prevent nesting and puts the action near the test.

try{
    if(IsValidFileFormat(filename) == false){
        DeleteFile(filename);
        LogError("invalid file format");
        return;
    }

    int folderID = GetFolderIDFromFilename(filename);
    if(folderID <= 0){
        DeleteFile(filename);
        LogError("invalid folder ID");
        return;
    }
    ...

}...

Answer 2

Guard clauses.

For each condition, negate it, change the else block into the then block, and return.

Thus

if(IsValidFileFormat(filename)
{
   // then
}
else
{
   // else
}

Becomes:

if(!IsValidFileFormat(filename)
{
    // else 
    return;     
}
// then

Answer 3

If you are not against using exceptions, you could handle the checks without nesting.

Warning, air code ahead:

public static void HandleUploadedFile(string filename)
{
  try
  {
    int folderID = GetFolderIDFromFilename(filename);

    if (folderID == 0)
      throw new InvalidFolderException("invalid folder ID");

    if (!IsValidFileFormat(filename))
      throw new InvalidFileException("invalid file format!");

    if (!HasNoViruses(filename))
      throw new VirusFoundException("failed virus test!");

    if (!VerifyFileSize(filename))
      throw new InvalidFileSizeException("file size invalid");

    // file is OK
    MoveToSafeFolder(filename);
  }
  catch (Exception ex)
  {
    DeleteFile(filename);
    LogError(ex.message);
  }
  finally
  {
    // do some things
  }
}

Answer 4

One possible approach is to have single if statements that check for when the condition isn't true. Have a return for each one of these checks. This turns your method into a sequence of 'if' blocks instead of a nest.