Пример рефакторинга кода на C#

Что такое рефакторинг? М. Фаулер дает следующее определение рефакторинга ("Рефакторинг: улучшение существующего кода"):

Рефакторинг (англ. refactoring) — процесс изменения внутренней структуры программы, не затрагивающий её внешнего поведения и имеющий целью облегчить понимание её работы. В основе рефакторинга лежит последовательность небольших эквивалентных (то есть сохраняющих поведение) преобразований.

Давайте рассмотрим на примере.

Задача:
В файл ODBC.INI добавить настройки принтера для устанавливаемой программы. Если настройки для данной программы уже имеются, то заменить их.
Имеется решение:

void InstallDriver(string drive)
{
    string PathToODBCINI = Environment.GetEnvironmentVariable("windir",  EnvironmentVariableTarget.Machine) + @"\ODBC.INI";
    if (!File.Exists(PathToODBCINI))
    {
       try
       {
          File.Create(PathToODBCINI).Close();
       }
       catch (Exception e)
       {
          MessageBox.Show(e.Message);
       }
    }
    StreamReader sr = new StreamReader(PathToODBCINI,    System.Text.ASCIIEncoding.Default);
    string content = sr.ReadToEnd();
    sr.Close();
    int index = content.IndexOf("[ODBC Data Sources]");
    if (index >= 0)
    {
       int lastIndex = content.IndexOf("QEWSD=34751", index) + 11;
       try
       {
           content = content.Remove(index, lastIndex - index);
       }
       catch
       {
           MessageBox.Show(ERROR_WRONG_PREVIOUS_INSTALLATION);
       }
    };
    string path_to_driver = Helpers.AppExecFolder + "files\\driver.txt";
    if (File.Exists(path_to_driver))
    {
        sr = new StreamReader(path_to_driver);
        string driver_text = sr.ReadToEnd();
        driver_text = driver_text.Replace("{1}", drive);
        sr.Close();
        try
        {
            StreamWriter sw = new StreamWriter(File.OpenWrite(PathToODBCINI), System.Text.ASCIIEncoding.Default);
            string config = String.Format("{0}{1}", driver_text, content);
            sw.Write(config);
            sw.Close();
            MessageBox.Show("Installation completed");
        }
        catch
        {
            MessageBox.Show(e1.Message);
        }
     }
     else
     {
        MessageBox.Show(String.Format("Cannot find file {0}", path_to_driver));
     }
}

Один метод делает всё... Хороший пример того, как не надо писать код. Сколько разных запахов тут смешалось воедино...
Давайте рассмотрим, что делает этот код, в виде последовательности действий:
0. Находит (или создает, если файл не существует) ODBC.INI
1. Получает содержимое ODBC.INI
2. Проверяет наличие настроек для программы и удаляет их
3. Формирует новые настройки
4. Записывает новые настройки в ODBC.INI
Давайте для начала разобъём этот метод на 5 отдельных методов в соответствии с выделенными действиями. Что получается:

string GetODBCINIPath()
{
    string _PathToODBCINI = Environment.GetEnvironmentVariable("windir", EnvironmentVariableTarget.Machine) + @"\ODBC.INI";
    if (!File.Exists(_PathToODBCINI))
    {
        File.Create(_PathToODBCINI).Close();
    }
    return _PathToODBCINI;
}
string GetODBCINIContent(string PathToODBCINI)
{
    using (StreamReader sr = new StreamReader(PathToODBCINI, System.Text.ASCIIEncoding.Default))
    {
        string content = sr.ReadToEnd();
        return content;
    }
}
string GetClearDriverSettings(string commonSettings)
{
    string clearSettings = commonSettings;
    int index = commonSettings.IndexOf("[ODBC Data Sources]");
    if (index >= 0)
    {
        int lastIndex = commonSettings.IndexOf("QEWSD=34751", index) + 11;
        clearSettings = commonSettings.Remove(index, lastIndex - index);
    };
    return clearSettings;
}
string MakeNewSettings(string drive, string oldSettings)
{
    string driver_text = String.Empty;
    string path_to_driver = Path.Combine(Helpers.AppExecFolder, "files\\driver.txt");
    if (File.Exists(path_to_driver))
    {
       using(StreamReader sr = new StreamReader(path_to_driver))
       {
          string driver_text = sr.ReadToEnd();
          driver_text = driver_text.Replace("{1}", drive);
       }
    }
    string newSettings = String.Concat(driver_text, oldSettings);
    return newSettings;
}
bool SetNewDriverSettings(string new_settings, string PathToODBCINI)
{
    bool result = true;
    using (StreamWriter sw = new StreamWriter(File.OpenWrite(PathToODBCINI), System.Text.ASCIIEncoding.Default))
    {
       try
       {
           sw.Write(new_settings);
       }
       catch
       {
           result = false;
       }
    }
    return result;
}
void InstallDriver(string drive)
{
    string PathToODBCINI = GetODBCINIPath();
    string commonSettings = GetODBCINIContent(PathToODBCINI);
    string clearSettings = GetClearDriverSettings(commonSettings);
    string newSettings = MakeNewSettings(drive, clearSettings);
    if (SetNewDriverSettings(newSettings, PathToODBCINI))
    {
        MessageBox.Show("Installation completed");
    }
    else
    {
        MessageBox.Show("Error");
    }
}

Теперь это выглядит немного лучше, но всё еще плохо. В глаза бросается PathToODBCINI, который используется в нескольких методах. Если присмотреться к этим методам, то мы увидим, что всех их объединяет работа с ODBC.INI, то есть логичнее будет выделить эти методы в отдельный класс. Заодно перенесем туда и назойливый PathToODBCINI. Получим:


class OdbcIniProvider
{
    private string PathToODBCINI;

    public OdbcIniProvider()
    {
        PathToODBCINI = GetODBCINIPath();
    }

    private string GetODBCINIPath()
    {
        string _PathToODBCINI = Environment.GetEnvironmentVariable("windir", EnvironmentVariableTarget.Machine) + @"\ODBC.INI";
        if (!File.Exists(_PathToODBCINI))
        {
            File.Create(_PathToODBCINI).Close();
        }
        return _PathToODBCINI;
    }

    public string GetODBCINIContent()
    {
       using (StreamReader sr = new StreamReader(PathToODBCINI, System.Text.ASCIIEncoding.Default))
       {
           return sr.ReadToEnd();
       }
    }

    public bool SetNewDriverSettings(string new_settings)
    {
       bool result = true;
       using (StreamWriter sw = new StreamWriter(File.OpenWrite(PathToODBCINI), System.Text.ASCIIEncoding.Default))
       {
           try
           {
               sw.Write(new_settings);
           }
           catch
           {
               result = false;
           }
       }
       return result;
    }
}
...
void InstallDriver(string drive)
{
   OdbcIniProvider odbcIniProvider = new OdbcIniProvider();
   string commonSettings = odbcIniProvider.GetODBCINIContent();
   string clearSettings = GetClearDriverSettings(commonSettings);
   string newSettings = MakeNewSettings(drive, clearSettings);
   if (odbcIniProvider.SetNewDriverSettings(newSettings))
   {
      MessageBox.Show("Installation completed");
   }
   else
   {
      MessageBox.Show("Error");
   }
}

Теперь присмотримся к оставшимся двум методам. Их тоже не мешало бы вынести в отдельный класс, который будет заниматься созданием и установкой новых настроек. Получим:

class SettingsManager
{
    public SettingsManager(){}

    public string GetClearDriverSettings(string commonSettings)
    {
        string clearSettings = commonSettings;
        int index = commonSettings.IndexOf("[ODBC Data Sources]");
        if (index >= 0)
        {
           int lastIndex = commonSettings.IndexOf("QEWSD=34751", index) + 11;
           clearSettings = commonSettings.Remove(index, lastIndex - index);
        };
        return clearSettings;
    }
    public string MakeNewSettings(string drive, string oldSettings)
    {
        string driver_text = String.Empty;
        string path_to_driver = Path.Combine(Helpers.AppExecFolder, "files\\driver.txt");
        if (File.Exists(path_to_driver))
        {
           using(StreamReader sr = new StreamReader(path_to_driver))
           {
              string driver_text = sr.ReadToEnd();
              driver_text = driver_text.Replace("{1}", drive);
           }
        }
        string newSettings = String.Concat(driver_text, oldSettings);
        return newSettings;
    }
}

Коренным образом ситуация не улучшилась. Мне совершенно не нравится, как взаимодействуют OdbcIniProvider и SettingsManager. Кроме того, SettingsManager имеет слишком много открытых методов, о которых вызывающему коду совершенно не обязательно знать. По сути, вызывающему коду достаточно одного метода SetNewSettings(string drive), т.е. :

class SettingsManager
{
   ...
   public bool SetNewSettings(string drive)
   {
      OdbcIniProvider odbcIniProvider = new OdbcIniProvider();
      string commonSettings = odbcIniProvider.GetODBCINIContent();
      string clearSettings = GetClearDriverSettings(commonSettings);
      string newSettings = MakeNewSettings(drive, clearSettings);
      return odbcIniProvider.SetNewDriverSettings(newSettings);
   }
}
...
void InstallDriver(string drive)
{
    SettingsManager settingsManager = new SettingsManager();

    if (settingsManager.SetNewSettings(drive))
    {
       MessageBox.Show("Installation completed");
    }
    else
    {
       MessageBox.Show("Error");
    }
}

Уже лучше, но такой подход тоже имеет свои недостатки. Например, что, если придется менять настройки не в файле, а в реестре? Придется переписывать SettingsManager, добавляя в него новый метод либо еще что-то в таком же духе... Давайте избавимся от такого рода зависимости сразу же. Для этого выделим интерфейс ISettingsProvider для классов, которые будут отвечать за конкретные способы получения и установки настроек:

interface ISettingsProvider
{
    string GetSettings();
    bool SetSettings(string new_settings);
}

И сразу же перепишем наш OdbcIniProvider под этот интерфейс:

class OdbcIniProvider : ISettingsProvider
{
    public string GetSettings() {...}
    public bool SetSettings(string new_settings) {...}
}

Теперь хотелось бы иметь возможность сконфигурировать SettingsManager на использование конкретного провайдера извне. Подправим немного класс SettingsManager, добавив в него метод для установки провайдера настроек:

class SettingsManager
{
    private ISettingsProvider settingsProvider;
    public void SetSettingsProvider(ISettingsProvider _settingsProvider)
    {
       settingsProvider = _settingsProvider;
    }
...
}

В итоге, получилось следующее:

class SettingsManager
{...
    public bool SetNewSettings(string drive)
    {
       string commonSettings = settingsProvider.GetSettings();
       string clearSettings = GetClearDriverSettings(commonSettings);
       string newSettings = MakeNewSettings(drive, clearSettings);
       return settingsProvider.SetSettings(newSettings);
    }
}
...
void InstallDriver(string drive)
{
    SettingsManager settingsManager = new SettingsManager();
    settingsManager.SetSettingsProvider(new OdbcIniProvider());
    if (settingsManager.SetNewSettings(drive))
    {
        MessageBox.Show("Installation completed");
    }
    else
    {
       MessageBox.Show("Error");
    }
}

Теперь выглядит лучше, но... а что, если форматирование настроек будет различаться? Просится, просится класс SettingsFormatter, реализующий интерфейс ISettingsFormatter... Пусть это будет домашним заданием ;)