Осуществите рефакторинг вложенный оператор IF для [закрытой] ясности

Если Вы не против использования исключений, Вы могли бы обработать проверки без вложения.

Предупреждение, воздушный кодекс вперед:

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
  }
}

Один возможный подход должен иметь единственный, если операторы, которые проверяют на то, когда условие не верно. Имейте возврат для каждой из этих проверок. Это превращает Ваш метод в последовательность 'если' блоки вместо вложенного множества.

Пункты охраны.

Для каждого условия инвертируйте его, еще изменитесь блок в тогдашний блок и возврат.

Таким образом

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

Становится:

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

В ответе David Waters мне не нравится повторный шаблон DeleteFile LogError. Я или записал бы вспомогательный метод под названием DeleteFileAndLog (строковый файл, строковая ошибка), или я напишу код как это:

public static void HandleUploadedFile(string filename)
{
    try
    {
        string errorMessage = TestForInvalidFile(filename);
        if (errorMessage != null)
        {
             LogError(errorMessage);
             DeleteFile(filename);
        }
        else
        {
            MoveToSafeFolder(filename);
        }
    }
    catch (Exception err)
    {
        LogError(err.Message);
        DeleteFile(filename);
    }
    finally { /* */ }
}

private static string TestForInvalidFile(filename)
{
    if (!IsValidFormat(filename))
        return "invalid file format.";
    if (!IsValidFolder(filename))
        return "invalid folder.";
    if (!IsVirusFree(filename))
        return "has viruses";
    if (!IsValidSize(filename))
        return "invalid size.";
    // ... etc ...
    return null;
 }

Нет много для рефакторинга здесь, поскольку Вы сохраняете 3 теста отдельно вследствие того, что сообщения об ошибках касаются выполненного теста. Вы могли выбрать то, что имели методы тестирования, сообщают ошибка зарегистрироваться так, у Вас нет их в, если/еще дерево, которое могло бы сделать вещи более простыми немного как Вы затем, могло просто протестировать на ошибку и зарегистрироваться, это + удаляет файл.

Это - elses выше того броска мой глаз. Вот альтернатива в попытке {}

Можно сделать это еще короче путем возврата после MoveToSafeFolder (Даже при том, что Вы возвращаетесь наконец, блок будет выполняться.) Затем Вы не должны присваивать пустую строку errorMessage, и Вы не должны проверять, errorString пустой прежде, чем удалить файл и зарегистрировать сообщение). Я не сделал этого здесь, потому что многие находят раннее наступление возвратов, и я согласился бы в этом экземпляре, начиная с наличия наконец, блок выполняется после того, как возврат неинтуитивен для многих людей.

Надеюсь, это поможет

            string errorMessage = "invalid file format";
            if (IsValidFileFormat(filename))
            {
                errorMessage = "invalid folder ID";
                int folderID = GetFolderIDFromFilename(filename);
                if (folderID > 0)
                {
                    errorMessage = "failed virus test";
                    if (HasNoViruses(filename))
                    {
                        errorMessage = "file size invalid";
                        if (VerifyFileSize(filename))
                        {
                            // file is OK                                        
                            MoveToSafeFolder(filename);
                            errorMessage = "";
                        }
                    }
                }
            }
            if (!string.IsNullOrEmpty(errorMessage))
            {
                DeleteFile(filename);
                LogError(errorMessage);
            }

Я был бы к чему-то вроде этого:

public enum FileStates {

MoveToSafeFolder = 1,

InvalidFileSize = 2,

FailedVirusTest = 3,

InvalidFolderID = 4,

InvalidFileFormat = 5,
}


public static void HandleUploadedFile(string filename) {
    try {
        switch (Handledoc(filename)) {
            case FileStates.FailedVirusTest:
                deletefile(filename);
                logerror("Virus");
                break;
            case FileStates.InvalidFileFormat:
                deletefile(filename);
                logerror("Invalid File format");
                break;
            case FileStates.InvalidFileSize:
                deletefile(filename);
                logerror("Invalid File Size");
                break;
            case FileStates.InvalidFolderID:
                deletefile(filename);
                logerror("Invalid Folder ID");
                break;
            case FileStates.MoveToSafeFolder:
                MoveToSafeFolder(filename);
                break;
        }
    }
    catch (Exception ex) {
        logerror("unknown error", ex.Message);
    }
}

private static FileStates Handledoc(string filename) {
    if (isvalidfileformat(filename)) {
        return FileStates.InvalidFileFormat;
    }
    if ((getfolderidfromfilename(filename) <= 0)) {
        return FileStates.InvalidFolderID;
    }
    if ((HasNoViruses(filename) == false)) {
        return FileStates.FailedVirusTest;
    }
    if ((VerifyFileSize(filename) == false)) {
        return FileStates.InvalidFileSize;
    }
    return FileStates.MoveToSafeFolder;
}

Как насчет этого?

public static void HandleUploadedFile(string filename)
{  
   try  
   {    
       if(!IsValidFileFormat(filename))        
       { DeleteAndLog(filename, "invalid file format"); return; }
       if(GetFolderIDFromFilename(filename)==0) 
       { DeleteAndLog(filename, "invalid folder ID");   return; }
       if(!HasNoViruses(filename))             
       { DeleteAndLog(filename, "failed virus test");   return; }
       if(!!VerifyFileSize(filename))          
       { DeleteAndLog(filename, "file size invalid");   return; }
       // --------------------------------------------------------
       MoveToSafeFolder(filename); 
   }
   catch (Exception ex)  { LogError("unknown error", ex.Message); throw; }
   finally {    // do some things  }
}     
private void DeleteAndLog(string fileName, string logMessage)
{
    DeleteFile(fileName);
    LogError(logMessage));
}

или, еще лучше... это:

public static void HandleUploadedFile(string filename)
{  
   try  
   {    
       if(ValidateUploadedFile(filename))       
           MoveToSafeFolder(filename); 
   }
   catch (Exception ex)  { LogError("unknown error", ex.Message); throw; }
   finally {    // do some things  }
} 
private bool ValidateUploadedFile(string fileName)
{
   if(!IsValidFileFormat(filename))        
    { DeleteAndLog(filename, "invalid file format"); return false; }
   if(GetFolderIDFromFilename(filename)==0) 
    { DeleteAndLog(filename, "invalid folder ID");   return false; }
   if(!HasNoViruses(filename))             
    { DeleteAndLog(filename, "failed virus test");   return false; }
   if(!!VerifyFileSize(filename))          
    { DeleteAndLog(filename, "file size invalid");   return false; }
   // ---------------------------------------------------------------
   return true;

}    
private void DeleteAndLog(string fileName, string logMessage)
{
    DeleteFile(fileName);
    LogError(logMessage));
}

Примечание: Вы не должны ловить и глотать универсальное Исключение, не повторно бросая его...